All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Adrian Hunter" <adrian.hunter@intel.com>
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	linux-pci@vger.kernel.org, Frank.Li@kernel.org,
	linux-i3c@lists.infradead.org
Subject: Re: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration
Date: Thu, 06 Aug 2026 13:39:14 +0000	[thread overview]
Message-ID: <20260806133914.EDA761F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260806131857.119830-2-adrian.hunter@intel.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dropping the bus maintenance lock around device_register() exposes the I3C device descriptor to concurrent Use-After-Free.

Pre-existing issues:
- [High] Unbalanced fwnode reference count decrement in i3c_device_release() leads to Use-After-Free.
--

--- Patch [1]: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration ---
commit b31a8482aa2ea710ccc853d0c74b77edebb131ff
Author: Adrian Hunter <adrian.hunter@intel.com>
Date:   Thu Aug 6 13:18:44 2026 +0000

    i3c: master: Fix recursive locking during device registration

This commit addresses a recursive locking lockdep warning by decoupling
device creation from registration. It collects newly populated devices into a
temporary list under the maintenance lock, then releases the lock prior to
invoking device_register() to prevent probe deadlocks. Finally, the lock is
re-acquired to clean up any devices that failed registration.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)

[ ... ]

> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> +	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);

[Severity: High]
Does dropping the bus maintenance lock around device_register() expose the
I3C device descriptor to a concurrent use-after-free?

During registration, the driver core can call subsystem functions like
i3c_device_match(), which read i3cdev->desc locklessly:

drivers/i3c/master.c:i3c_device_match() {
    ...
    if (i3cdev->desc && i3cdev->desc->boardinfo)
        static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
    ...
}

Concurrently, an I3C Hot-Join or sysfs-triggered Dynamic Address Assignment
(DAA) could execute i3c_master_do_daa(), take the i3c_bus_maintenance_lock,
and call __i3c_master_add_i3c_dev_locked(). If DAA detects the device as a
duplicate, it replaces the descriptor and frees the old one:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
    ...
    i3c_master_detach_i3c_dev(olddev);
    i3c_master_free_i3c_dev(olddev);
    ...
}

If the registering thread is actively evaluating i3cdev->desc when this
happens, will it lead to memory corruption when reading fields like
desc->boardinfo?

> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);
> +	}
> +
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		list_del(&i3cdev->node);
> +		desc = i3cdev->desc;
> +		i3cdev->desc = NULL;
> +		put_device(&i3cdev->dev);

[Severity: High]
This is a pre-existing issue, but does this error path cause an unbalanced
fwnode reference count decrement leading to a use-after-free?

During device initialization, device_set_node() assigns the fwnode without
incrementing its reference count:

drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
    ...
    if (desc->boardinfo)
        device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
    ...
}

However, when put_device() is called here on registration failure,
the release function unconditionally drops a reference:

drivers/i3c/master.c:i3c_device_release() {
    ...
    fwnode_handle_put(dev->fwnode);
    ...
}

Since the device object never acquired a reference to the boardinfo's fwnode,
will repeated registration failures or unbind operations drop the refcount
below zero, eventually prematurely freeing the fwnode?

> +		desc->dev = NULL;
>  	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=1

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Adrian Hunter" <adrian.hunter@intel.com>
Cc: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	linux-pci@vger.kernel.org, Frank.Li@kernel.org,
	linux-i3c@lists.infradead.org
Subject: Re: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration
Date: Thu, 06 Aug 2026 13:39:14 +0000	[thread overview]
Message-ID: <20260806133914.EDA761F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260806131857.119830-2-adrian.hunter@intel.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Dropping the bus maintenance lock around device_register() exposes the I3C device descriptor to concurrent Use-After-Free.

Pre-existing issues:
- [High] Unbalanced fwnode reference count decrement in i3c_device_release() leads to Use-After-Free.
--

--- Patch [1]: [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration ---
commit b31a8482aa2ea710ccc853d0c74b77edebb131ff
Author: Adrian Hunter <adrian.hunter@intel.com>
Date:   Thu Aug 6 13:18:44 2026 +0000

    i3c: master: Fix recursive locking during device registration

This commit addresses a recursive locking lockdep warning by decoupling
device creation from registration. It collects newly populated devices into a
temporary list under the maintenance lock, then releases the lock prior to
invoking device_register() to prevent probe deadlocks. Finally, the lock is
re-acquired to clean up any devices that failed registration.

> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)

[ ... ]

> +		list_add_tail(&desc->dev->node, &i3c_unreg_devs);
> +	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		ret = device_register(&i3cdev->dev);

[Severity: High]
Does dropping the bus maintenance lock around device_register() expose the
I3C device descriptor to a concurrent use-after-free?

During registration, the driver core can call subsystem functions like
i3c_device_match(), which read i3cdev->desc locklessly:

drivers/i3c/master.c:i3c_device_match() {
    ...
    if (i3cdev->desc && i3cdev->desc->boardinfo)
        static_addr_method = i3cdev->desc->boardinfo->static_addr_method;
    ...
}

Concurrently, an I3C Hot-Join or sysfs-triggered Dynamic Address Assignment
(DAA) could execute i3c_master_do_daa(), take the i3c_bus_maintenance_lock,
and call __i3c_master_add_i3c_dev_locked(). If DAA detects the device as a
duplicate, it replaces the descriptor and frees the old one:

drivers/i3c/master.c:__i3c_master_add_i3c_dev_locked() {
    ...
    i3c_master_detach_i3c_dev(olddev);
    i3c_master_free_i3c_dev(olddev);
    ...
}

If the registering thread is actively evaluating i3cdev->desc when this
happens, will it lead to memory corruption when reading fields like
desc->boardinfo?

> +		if (ret)
> +			dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
> +		else
> +			list_del_init(&i3cdev->node);
> +	}
> +
> +	i3c_bus_maintenance_lock(&master->bus);
> +
> +	list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
> +		list_del(&i3cdev->node);
> +		desc = i3cdev->desc;
> +		i3cdev->desc = NULL;
> +		put_device(&i3cdev->dev);

[Severity: High]
This is a pre-existing issue, but does this error path cause an unbalanced
fwnode reference count decrement leading to a use-after-free?

During device initialization, device_set_node() assigns the fwnode without
incrementing its reference count:

drivers/i3c/master.c:i3c_master_register_new_i3c_devs() {
    ...
    if (desc->boardinfo)
        device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
    ...
}

However, when put_device() is called here on registration failure,
the release function unconditionally drops a reference:

drivers/i3c/master.c:i3c_device_release() {
    ...
    fwnode_handle_put(dev->fwnode);
    ...
}

Since the device object never acquired a reference to the boardinfo's fwnode,
will repeated registration failures or unbind operations drop the refcount
below zero, eventually prematurely freeing the fwnode?

> +		desc->dev = NULL;
>  	}
> +
> +	i3c_bus_maintenance_unlock(&master->bus);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806131857.119830-1-adrian.hunter@intel.com?part=1

  reply	other threads:[~2026-08-06 13:39 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 13:18 [PATCH V4 00/14] i3c: Support IBI-based system wakeup Adrian Hunter
2026-08-06 13:18 ` Adrian Hunter
2026-08-06 13:18 ` [PATCH V4 01/14] i3c: master: Fix recursive locking during device registration Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:39   ` sashiko-bot [this message]
2026-08-06 13:39     ` sashiko-bot
2026-08-06 16:46     ` Adrian Hunter
2026-08-06 16:46       ` Adrian Hunter
2026-08-06 18:24   ` Frank Li
2026-08-06 18:24     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 02/14] i3c: Fix unlocked dereference of dev->desc in i3c_device_get_supported_xfer_mode() Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:38   ` sashiko-bot
2026-08-06 13:38     ` sashiko-bot
2026-08-06 18:26   ` Frank Li
2026-08-06 18:26     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 03/14] i3c: master: Do not treat master device as a duplicate target Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:40   ` sashiko-bot
2026-08-06 13:40     ` sashiko-bot
2026-08-06 13:18 ` [PATCH V4 04/14] i3c: master: Fix use-after-free of master->this Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:39   ` sashiko-bot
2026-08-06 13:39     ` sashiko-bot
2026-08-06 18:30   ` Frank Li
2026-08-06 18:30     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 05/14] i3c: Make dev->desc locking assumptions explicit Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:39   ` sashiko-bot
2026-08-06 13:39     ` sashiko-bot
2026-08-06 18:31   ` Frank Li
2026-08-06 18:31     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 06/14] i3c: master: Fix potential UAF in i3c_device_uevent() Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:34   ` sashiko-bot
2026-08-06 13:34     ` sashiko-bot
2026-08-06 18:33   ` Frank Li
2026-08-06 18:33     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 07/14] i3c: master: Fix potential UAF in i3c_device_match() Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:45   ` sashiko-bot
2026-08-06 13:45     ` sashiko-bot
2026-08-06 17:13     ` Adrian Hunter
2026-08-06 17:13       ` Adrian Hunter
2026-08-06 13:18 ` [PATCH V4 08/14] i3c: master: Support IBI-based wakeup capability Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:31   ` sashiko-bot
2026-08-06 13:31     ` sashiko-bot
2026-08-06 18:58   ` Mukesh Savaliya
2026-08-06 18:58     ` Mukesh Savaliya
2026-08-06 13:18 ` [PATCH V4 09/14] i3c: master: Report wakeup events for IBIs Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:49   ` sashiko-bot
2026-08-06 13:49     ` sashiko-bot
2026-08-06 17:16     ` Adrian Hunter
2026-08-06 17:16       ` Adrian Hunter
2026-08-06 18:35   ` Frank Li
2026-08-06 18:35     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 10/14] i3c: master: Add helper to query bus wakeup requirements Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:29   ` sashiko-bot
2026-08-06 13:29     ` sashiko-bot
2026-08-06 18:46   ` Frank Li
2026-08-06 18:46     ` Frank Li
2026-08-06 19:03   ` Mukesh Savaliya
2026-08-06 19:03     ` Mukesh Savaliya
2026-08-06 13:18 ` [PATCH V4 11/14] i3c: master: Reject IBI requests from non-IBI-capable devices Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:41   ` sashiko-bot
2026-08-06 13:41     ` sashiko-bot
2026-08-06 13:18 ` [PATCH V4 12/14] i3c: mipi-i3c-hci-pci: Propagate I3C wakeup requirements to PCI Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:56   ` sashiko-bot
2026-08-06 13:56     ` sashiko-bot
2026-08-06 17:20     ` Adrian Hunter
2026-08-06 17:20       ` Adrian Hunter
2026-08-06 18:42   ` Frank Li
2026-08-06 18:42     ` Frank Li
2026-08-06 13:18 ` [PATCH V4 13/14] i3c: mipi-i3c-hci: Factor out i3c_hci_sysdev() Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:46   ` sashiko-bot
2026-08-06 13:46     ` sashiko-bot
2026-08-06 18:44   ` Frank Li
2026-08-06 18:44     ` Frank Li
2026-08-06 19:07   ` Mukesh Savaliya
2026-08-06 19:07     ` Mukesh Savaliya
2026-08-06 13:18 ` [PATCH V4 14/14] i3c: mipi-i3c-hci: Advertise IBI wakeup capability Adrian Hunter
2026-08-06 13:18   ` Adrian Hunter
2026-08-06 13:54   ` sashiko-bot
2026-08-06 13:54     ` sashiko-bot
2026-08-06 17:37     ` Adrian Hunter
2026-08-06 17:37       ` Adrian Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806133914.EDA761F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=linux-i3c@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.