Re: [PATCH v1] resource: report a devm_request_resource() conflict under the lock

[Bradley Morgan]

On 16 October 2026 15:00:43 BST, Nickolai Zeldovich
<nickolai@xxxxxxxxxxxxx> wrote:
>devm_request_resource() calls request_resource_conflict(), which takes
>resource_lock, tries to insert the new resource and returns a pointer to
>the conflicting resource after dropping the lock.  The error message is
>then formatted from that pointer: conflict->name and the %pR argument
>read the name, start, end, flags and desc of a resource that nothing
>pins any more.  The owner of the conflicting resource may call
>release_resource() and free it the moment the lock is dropped.
>
>The conflicting resource normally belongs to another device.  With two
>devices whose drivers claim overlapping regions (two PCI host bridges
>whose "ranges" windows overlap, two instances of a controller on the
>same aperture, or a stale claim an administrator is replacing), the
>following interleaving reads freed memory:
>
>  CPU 0: driver B probe                  CPU 1: driver A unbind
>  devm_request_resource(devB, root, resB)
>    request_resource_conflict()
>      write_lock(&resource_lock)
>      __request_resource() -> resA
>      write_unlock(&resource_lock)
>                                         devres_release_all(devA)
>                                           devm_resource_release()
>                                             release_resource(resA)
>                                         put_device() -> kfree(resA)
>    dev_err(..., resA->name, resA)        use after free
>
>Both halves are reachable from userspace on any kernel: a write to the
>"unbind" file of the driver that holds the region and a write to the
>"bind" file of the driver that wants it.  One in-tree path is
>drivers/pci/bus.c devm_request_pci_bus_resources(), called from host
>bridge probe for every bridge window; the windows are the embedded
>struct resource of kzalloc'ed resource_entry objects that
>pci_free_resource_list() frees when the other bridge is released.
>In the common case the "resource collision" message is garbled; with
>the slab object poisoned (CONFIG_SLUB_DEBUG) or reused, formatting
>conflict->name oopses, and KASAN reports a slab-use-after-free read in
>devm_request_resource().  All of this happens in a probe that is
>already failing with -EBUSY.  The window is a few hundred instructions
>wide and exists only when two devices actually collide, so the race
>has not been observed outside of analysis.
>
>Do the same here: take resource_lock, insert with __request_resource()
>and print the diagnostic while the lock still keeps the conflicting
>resource in the tree, then drop the lock.  The return values and the
>devres handling are unchanged.
>
>The change was built for x86_64 and RISC-V; the resource KUnit tests
>pass on both.
>
>Found by formal verification of kernel/resource.c against the kernel
>binary.

Ok.

>
>Fixes: 8d38821cbcf5 ("resources: Add device-managed request/release_resource()")
>Assisted-by: LLM

Fair enough.

>Signed-off-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
>---
>
>Notes:
>    The same read-after-unlock shape exists in every other caller of
>    request_resource_conflict() and insert_resource_conflict():
>    pci_claim_resource(), pci_revert_fw_address(), pci_bus_insert_busn_res(),
>    pci_ecam_create(), acpi_pci_root_validate_resources(),
>    efi_mem_reserve_iomem() and the sparc, x86, loongarch and powerpc arch
>    callers all format the returned pointer after the lock is gone.  Those
>    run at boot or from PCI enumeration and live outside kernel/resource.c,
>    where resource_lock is not visible, so this patch only changes the
>    driver-facing probe() entry point that pairs with an unbind of the
>    other device.  If a treewide answer is wanted, a request variant that
>    reports the conflict from under the lock (or takes a callback) would be
>    the shape, and devm_request_resource() would be its first user.
>    
>    Testing: the resource KUnit suite (CONFIG_RESOURCE_KUNIT_TEST) was run
>    under QEMU before and after on x86_64 and RISC-V, 3/3 passing in all
>    four runs; defconfig builds for both architectures with GCC are
>    warning-free, and kernel/resource.o builds clean with W=1 and sparse on
>    both.
>
> kernel/resource.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/kernel/resource.c b/kernel/resource.c
>index e60539a55541..08bd7579cf40 100644
>--- a/kernel/resource.c
>+++ b/kernel/resource.c
>@@ -1695,10 +1695,15 @@ int devm_request_resource(struct device *dev, struct resource *root,
> 
> 	*ptr = new;
> 
>-	conflict = request_resource_conflict(root, new);
>-	if (conflict) {
>+	/* Report a conflict while the lock still keeps it in the tree. */
>+	write_lock(&resource_lock);
>+	conflict = __request_resource(root, new);
>+	if (conflict)
> 		dev_err(dev, "resource collision: %pR conflicts with %s %pR\n",
> 			new, conflict->name, conflict);
>+	write_unlock(&resource_lock);
>+
>+	if (conflict) {
> 		devres_free(ptr);
> 		return -EBUSY;
> 	}

Looks fine to me, thanks for the patch:

Reviewed-by: Bradley Morgan <brads@xxxxxxxxxxxxxx>


>
>base-commit: 90201351a862182bdabfc1ad1c97b1e1b6ed5931
>

--- Thanks!
"I'm not a very positive person" - Linus torvalds