As part of I3C driver probing sequence for particular device instance, While adding to queue it is trying to access ibi variable of dev which is not yet initialized causing "Unable to handle kernel read from unreadable memory" resulting in kernel panic.
Below is the sequence where this issue happened. 1. During boot up sequence IBI is received at host from the slave device before requesting for IBI, Usually will request IBI by calling i3c_device_request_ibi() during probe of slave driver. 2. Since master code trying to access IBI Variable for the particular device instance before actually it initialized by slave driver, due to this randomly accessing the address and causing kernel panic. 3. i3c_device_request_ibi() function invoked by the slave driver where dev->ibi = ibi; assigned as part of function call i3c_dev_request_ibi_locked(). 4. But when IBI request sent by slave device, master code trying to access this variable before its initialized due to this race condition situation kernel panic happened.
Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/Z9gjGYudiYyl3bSe@lizhi-Precision-Tower-5810/ Signed-off-by: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com --- Changes since v4: - Fix added at generic places master.c which is applicable for all platforms
drivers/i3c/master.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index d5dc4180afbc..c65006aa0684 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2561,6 +2561,9 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master) */ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) { + if (!dev->ibi || !slot) + return; + atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work); }
Subject should be
i3c: Add NULL pointer check in i3c_master_queue_ibi()
On Tue, Mar 25, 2025 at 03:53:32PM +0530, Manjunatha Venkatesh wrote:
As part of I3C driver probing sequence for particular device instance, While adding to queue it is trying to access ibi variable of dev which is not yet initialized causing "Unable to handle kernel read from unreadable memory" resulting in kernel panic.
Below is the sequence where this issue happened.
- During boot up sequence IBI is received at host from the slave device before requesting for IBI, Usually will request IBI by calling i3c_device_request_ibi() during probe of slave driver.
- Since master code trying to access IBI Variable for the particular device instance before actually it initialized by slave driver, due to this randomly accessing the address and causing kernel panic.
- i3c_device_request_ibi() function invoked by the slave driver where dev->ibi = ibi; assigned as part of function call i3c_dev_request_ibi_locked().
- But when IBI request sent by slave device, master code trying to access this variable before its initialized due to this race condition situation kernel panic happened.
How about commit message as:
The I3C master driver may receive an IBI from a target device that has not been probed yet. In such cases, the master calls `i3c_master_queue_ibi()` to queue an IBI work task, leading to "Unable to handle kernel read from unreadable memory" and resulting in a kernel panic.
Typical IBI handling flow: 1. The I3C master scans target devices and probes their respective drivers. 2. The target device driver calls `i3c_device_request_ibi()` to enable IBI and assigns `dev->ibi = ibi`. 3. The I3C master receives an IBI from the target device and calls `i3c_master_queue_ibi()` to queue the target device driver’s IBI handler task.
However, since target device events are asynchronous to the I3C probe sequence, step 3 may occur before step 2, causing `dev->ibi` to be `NULL`, leading to a kernel panic.
Add a NULL pointer check in `i3c_master_queue_ibi()` to prevent accessing an uninitialized `dev->ibi`, ensuring stability.
Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/Z9gjGYudiYyl3bSe@lizhi-Precision-Tower-5810/ Signed-off-by: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com
Changes since v4:
- Fix added at generic places master.c which is applicable for all platforms
drivers/i3c/master.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index d5dc4180afbc..c65006aa0684 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2561,6 +2561,9 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master) */ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) {
- if (!dev->ibi || !slot)
return;- atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work);
}
2.46.1
-----Original Message----- From: Frank Li frank.li@nxp.com Sent: Tuesday, March 25, 2025 8:13 PM To: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com Cc: alexandre.belloni@bootlin.com; arnd@arndb.de; gregkh@linuxfoundation.org; bbrezillon@kernel.org; linux- i3c@lists.infradead.org; linux-kernel@vger.kernel.org; rvmanjumce@gmail.com; stable@vger.kernel.org Subject: Re: [PATCH v5] i3c: Fix read from unreadable memory at i3c_master_queue_ibi()
Subject should be
i3c: Add NULL pointer check in i3c_master_queue_ibi()
[Manjunatha Venkatesh] : I will update the subject line in the next commit.
On Tue, Mar 25, 2025 at 03:53:32PM +0530, Manjunatha Venkatesh wrote:
As part of I3C driver probing sequence for particular device instance, While adding to queue it is trying to access ibi variable of dev which is not yet initialized causing "Unable to handle kernel read from unreadable memory" resulting in kernel panic.
Below is the sequence where this issue happened.
- During boot up sequence IBI is received at host from the slave device before requesting for IBI, Usually will request IBI by calling i3c_device_request_ibi() during probe of slave driver.
- Since master code trying to access IBI Variable for the particular device instance before actually it initialized by slave driver, due to this randomly accessing the address and causing kernel panic.
- i3c_device_request_ibi() function invoked by the slave driver where dev->ibi = ibi; assigned as part of function call i3c_dev_request_ibi_locked().
- But when IBI request sent by slave device, master code trying to access this variable before its initialized due to this race condition situation kernel panic happened.
How about commit message as:
The I3C master driver may receive an IBI from a target device that has not been probed yet. In such cases, the master calls `i3c_master_queue_ibi()` to queue an IBI work task, leading to "Unable to handle kernel read from unreadable memory" and resulting in a kernel panic.
Typical IBI handling flow:
- The I3C master scans target devices and probes their respective drivers.
- The target device driver calls `i3c_device_request_ibi()` to enable IBI and assigns `dev->ibi = ibi`.
- The I3C master receives an IBI from the target device and calls `i3c_master_queue_ibi()` to queue the target device driver’s IBI handler task.
However, since target device events are asynchronous to the I3C probe sequence, step 3 may occur before step 2, causing `dev->ibi` to be `NULL`, leading to a kernel panic.
Add a NULL pointer check in `i3c_master_queue_ibi()` to prevent accessing an uninitialized `dev->ibi`, ensuring stability.
[Manjunatha Venkatesh] : This commit message looks better, will use the same in the next commit.
Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/Z9gjGYudiYyl3bSe@lizhi-Precision-Tower-58 10/ Signed-off-by: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com
Changes since v4:
- Fix added at generic places master.c which is applicable for all
platforms
drivers/i3c/master.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index d5dc4180afbc..c65006aa0684 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2561,6 +2561,9 @@ static void i3c_master_unregister_i3c_devs(struct
i3c_master_controller *master)
*/ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) {
- if (!dev->ibi || !slot)
return;- atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work); }
-- 2.46.1
On 3/25/2025 8:13 PM, Frank Li wrote:
Subject should be
i3c: Add NULL pointer check in i3c_master_queue_ibi()
yes, Aligned.
On Tue, Mar 25, 2025 at 03:53:32PM +0530, Manjunatha Venkatesh wrote:
As part of I3C driver probing sequence for particular device instance, While adding to queue it is trying to access ibi variable of dev which is not yet initialized causing "Unable to handle kernel read from unreadable memory" resulting in kernel panic.
Below is the sequence where this issue happened.
- During boot up sequence IBI is received at host from the slave device before requesting for IBI, Usually will request IBI by calling i3c_device_request_ibi() during probe of slave driver.
- Since master code trying to access IBI Variable for the particular device instance before actually it initialized by slave driver, due to this randomly accessing the address and causing kernel panic.
- i3c_device_request_ibi() function invoked by the slave driver where dev->ibi = ibi; assigned as part of function call i3c_dev_request_ibi_locked().
- But when IBI request sent by slave device, master code trying to access this variable before its initialized due to this race condition situation kernel panic happened.
How about commit message as:
The I3C master driver may receive an IBI from a target device that has not been probed yet. In such cases, the master calls `i3c_master_queue_ibi()` to queue an IBI work task, leading to "Unable to handle kernel read from unreadable memory" and resulting in a kernel panic.
Typical IBI handling flow:
- The I3C master scans target devices and probes their respective drivers.
- The target device driver calls `i3c_device_request_ibi()` to enable IBI and assigns `dev->ibi = ibi`.
- The I3C master receives an IBI from the target device and calls `i3c_master_queue_ibi()` to queue the target device driver’s IBI handler task.
However, since target device events are asynchronous to the I3C probe sequence, step 3 may occur before step 2, causing `dev->ibi` to be `NULL`, leading to a kernel panic.
Add a NULL pointer check in `i3c_master_queue_ibi()` to prevent accessing an uninitialized `dev->ibi`, ensuring stability.
Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/Z9gjGYudiYyl3bSe@lizhi-Precision-Tower-5810/ Signed-off-by: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com
Changes since v4:
- Fix added at generic places master.c which is applicable for all platforms
drivers/i3c/master.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index d5dc4180afbc..c65006aa0684 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2561,6 +2561,9 @@ static void i3c_master_unregister_i3c_devs(struct i3c_master_controller *master) */ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) {
- if (!dev->ibi || !slot)
return;
1. Shouldn't this be a Logical AND ? what if slot is non NULL but the IBI is NULL ?
2. This being void function, it doesn't say anything to caller if it's successful or failed ? Should we make this non void function ? if not, i am thinking it may run into multiple attempts, no log too.
atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work); } -- 2.46.1
-----Original Message----- From: Mukesh Kumar Savaliya quic_msavaliy@quicinc.com Sent: Wednesday, March 26, 2025 12:15 PM To: Frank Li frank.li@nxp.com; Manjunatha Venkatesh manjunatha.venkatesh@nxp.com Cc: alexandre.belloni@bootlin.com; arnd@arndb.de; gregkh@linuxfoundation.org; bbrezillon@kernel.org; linux- i3c@lists.infradead.org; linux-kernel@vger.kernel.org; rvmanjumce@gmail.com; stable@vger.kernel.org Subject: [EXT] Re: [PATCH v5] i3c: Fix read from unreadable memory at i3c_master_queue_ibi()
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
On 3/25/2025 8:13 PM, Frank Li wrote:
Subject should be
i3c: Add NULL pointer check in i3c_master_queue_ibi()
yes, Aligned.
On Tue, Mar 25, 2025 at 03:53:32PM +0530, Manjunatha Venkatesh wrote:
As part of I3C driver probing sequence for particular device instance, While adding to queue it is trying to access ibi variable of dev which is not yet initialized causing "Unable to handle kernel read from unreadable memory" resulting in kernel panic.
Below is the sequence where this issue happened.
- During boot up sequence IBI is received at host from the slave device before requesting for IBI, Usually will request IBI by calling i3c_device_request_ibi() during probe of slave driver.
- Since master code trying to access IBI Variable for the particular device instance before actually it initialized by slave driver, due to this randomly accessing the address and causing kernel panic.
- i3c_device_request_ibi() function invoked by the slave driver where dev->ibi = ibi; assigned as part of function call i3c_dev_request_ibi_locked().
- But when IBI request sent by slave device, master code trying to access this variable before its initialized due to this race condition situation kernel panic happened.
How about commit message as:
The I3C master driver may receive an IBI from a target device that has not been probed yet. In such cases, the master calls `i3c_master_queue_ibi()` to queue an IBI work task, leading to "Unable to handle kernel read from unreadable memory" and resulting in a kernel
panic.
Typical IBI handling flow:
- The I3C master scans target devices and probes their respective drivers.
- The target device driver calls `i3c_device_request_ibi()` to enable IBI and assigns `dev->ibi = ibi`.
- The I3C master receives an IBI from the target device and calls `i3c_master_queue_ibi()` to queue the target device driver's IBI handler task.
However, since target device events are asynchronous to the I3C probe sequence, step 3 may occur before step 2, causing `dev->ibi` to be `NULL`, leading to a kernel panic.
Add a NULL pointer check in `i3c_master_queue_ibi()` to prevent accessing an uninitialized `dev->ibi`, ensuring stability.
Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure") Cc: stable@vger.kernel.org Link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flor e.kernel.org%2Flkml%2FZ9gjGYudiYyl3bSe%40lizhi-Precision-Tower-
5810%2
F&data=05%7C02%7Cmanjunatha.venkatesh%40nxp.com%7Ceda8be8c7abc4 3b4ab9
608dd6c31c8e7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6387 856832
77582903%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYi OiIwLjAu
MDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C% 7C%7C
&sdata=oyc9Wv%2Fj8HMRFIiIGL9nIw0XvI6FsLK2SvsQJ55H7XI%3D&reserved= 0
Signed-off-by: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com
Changes since v4:
- Fix added at generic places master.c which is applicable for all
platforms
drivers/i3c/master.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index d5dc4180afbc..c65006aa0684 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2561,6 +2561,9 @@ static void
i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
*/ void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot
*slot)
{
- if (!dev->ibi || !slot)
return;
- Shouldn't this be a Logical AND ? what if slot is non NULL but the IBI is
NULL ?
[Manjunatha Venkatesh] : I think Logical OR operation is correct, Since if any one of the variable is NULL need to return before accessing those variables.
- This being void function, it doesn't say anything to caller if it's successful or
failed ? Should we make this non void function ? if not, i am thinking it may run into multiple attempts, no log too.
[Manjunatha Venkatesh] : If required we can add error print log, Others please confirm your opinion on this.
atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work);}
2.46.1
On 3/26/2025 12:28 PM, Manjunatha Venkatesh wrote:
-----Original Message----- From: Mukesh Kumar Savaliya quic_msavaliy@quicinc.com Sent: Wednesday, March 26, 2025 12:15 PM To: Frank Li frank.li@nxp.com; Manjunatha Venkatesh manjunatha.venkatesh@nxp.com Cc: alexandre.belloni@bootlin.com; arnd@arndb.de; gregkh@linuxfoundation.org; bbrezillon@kernel.org; linux- i3c@lists.infradead.org; linux-kernel@vger.kernel.org; rvmanjumce@gmail.com; stable@vger.kernel.org Subject: [EXT] Re: [PATCH v5] i3c: Fix read from unreadable memory at i3c_master_queue_ibi()
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
On 3/25/2025 8:13 PM, Frank Li wrote:
Subject should be
i3c: Add NULL pointer check in i3c_master_queue_ibi()
yes, Aligned.
On Tue, Mar 25, 2025 at 03:53:32PM +0530, Manjunatha Venkatesh wrote:
As part of I3C driver probing sequence for particular device instance, While adding to queue it is trying to access ibi variable of dev which is not yet initialized causing "Unable to handle kernel read from unreadable memory" resulting in kernel panic.
Below is the sequence where this issue happened.
- During boot up sequence IBI is received at host from the slave device before requesting for IBI, Usually will request IBI by calling i3c_device_request_ibi() during probe of slave driver.
- Since master code trying to access IBI Variable for the particular device instance before actually it initialized by slave driver, due to this randomly accessing the address and causing kernel panic.
- i3c_device_request_ibi() function invoked by the slave driver where dev->ibi = ibi; assigned as part of function call i3c_dev_request_ibi_locked().
- But when IBI request sent by slave device, master code trying to access this variable before its initialized due to this race condition situation kernel panic happened.
How about commit message as:
The I3C master driver may receive an IBI from a target device that has not been probed yet. In such cases, the master calls `i3c_master_queue_ibi()` to queue an IBI work task, leading to "Unable to handle kernel read from unreadable memory" and resulting in a kernel
panic.
Typical IBI handling flow:
- The I3C master scans target devices and probes their respective drivers.
- The target device driver calls `i3c_device_request_ibi()` to enable IBI and assigns `dev->ibi = ibi`.
- The I3C master receives an IBI from the target device and calls `i3c_master_queue_ibi()` to queue the target device driver's IBI handler task.
However, since target device events are asynchronous to the I3C probe sequence, step 3 may occur before step 2, causing `dev->ibi` to be `NULL`, leading to a kernel panic.
Add a NULL pointer check in `i3c_master_queue_ibi()` to prevent accessing an uninitialized `dev->ibi`, ensuring stability.
Fixes: 3a379bbcea0af ("i3c: Add core I3C infrastructure") Cc: stable@vger.kernel.org Link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flor e.kernel.org%2Flkml%2FZ9gjGYudiYyl3bSe%40lizhi-Precision-Tower-
5810%2
F&data=05%7C02%7Cmanjunatha.venkatesh%40nxp.com%7Ceda8be8c7abc4 3b4ab9
608dd6c31c8e7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6387 856832
77582903%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYi OiIwLjAu
MDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C% 7C%7C
&sdata=oyc9Wv%2Fj8HMRFIiIGL9nIw0XvI6FsLK2SvsQJ55H7XI%3D&reserved= 0
Signed-off-by: Manjunatha Venkatesh manjunatha.venkatesh@nxp.com
Changes since v4: - Fix added at generic places master.c which is applicable for all platforms
drivers/i3c/master.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c index d5dc4180afbc..c65006aa0684 100644 --- a/drivers/i3c/master.c +++ b/drivers/i3c/master.c @@ -2561,6 +2561,9 @@ static void
i3c_master_unregister_i3c_devs(struct i3c_master_controller *master)
*/void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot
*slot)
{
- if (!dev->ibi || !slot)
return;
- Shouldn't this be a Logical AND ? what if slot is non NULL but the IBI is
NULL ?
[Manjunatha Venkatesh] : I think Logical OR operation is correct, Since if any one of the variable is NULL need to return before accessing those variables.
I agree, i was incorrect.
- This being void function, it doesn't say anything to caller if it's successful or
failed ? Should we make this non void function ? if not, i am thinking it may run into multiple attempts, no log too.
[Manjunatha Venkatesh] : If required we can add error print log, Others please confirm your opinion on this.
atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work);}
2.46.1
linux-stable-mirror@lists.linaro.org
-
Frank Li -
Manjunatha Venkatesh -
Mukesh Kumar Savaliya