[PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Manjunatha Venkatesh]

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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C master driver")
Cc: stable@vger.kernel.org
Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
---
Changes since v3:
  - Description  updated typo "Fixes:"

 drivers/i3c/master/svc-i3c-master.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index d6057d8c7dec..98c4d2e5cd8d 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -534,8 +534,11 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
 	switch (ibitype) {
 	case SVC_I3C_MSTATUS_IBITYPE_IBI:
 		if (dev) {
-			i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
-			master->ibi.tbq_slot = NULL;
+			data = i3c_dev_get_master_data(dev);
+			if (master->ibi.slots[data->ibi]) {
+				i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
+				master->ibi.tbq_slot = NULL;
+			}
 		}
 		svc_i3c_master_emit_stop(master);
 		break;
-- 
2.46.1

Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Frank Li]

On Wed, Mar 12, 2025 at 07:23:56PM +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.
> 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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C master driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> ---
> Changes since v3:
>   - Description  updated typo "Fixes:"
>
>  drivers/i3c/master/svc-i3c-master.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index d6057d8c7dec..98c4d2e5cd8d 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -534,8 +534,11 @@ static void svc_i3c_master_ibi_work(struct work_struct *work)
>  	switch (ibitype) {
>  	case SVC_I3C_MSTATUS_IBITYPE_IBI:
>  		if (dev) {
> -			i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> -			master->ibi.tbq_slot = NULL;
> +			data = i3c_dev_get_master_data(dev);
> +			if (master->ibi.slots[data->ibi]) {
> +				i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> +				master->ibi.tbq_slot = NULL;
> +			}

You still not reply previous discussion:

https://lore.kernel.org/linux-i3c/Z8sOKZSjHeeP2mY5@lizhi-Precision-Tower-5810/T/#mfd02d6ddca0a4b57bc823dcbfa7571c564800417

This is not issue only at svc driver, which should be common problem for
other master controller drivers

Frank

>  		}
>  		svc_i3c_master_emit_stop(master);
>  		break;
> --
> 2.46.1
>

RE: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Manjunatha Venkatesh]

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Wednesday, March 12, 2025 11:59 PM
> To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at
> svc_i3c_master_ibi_work()
> 
> On Wed, Mar 12, 2025 at 07:23:56PM +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.
> > 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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C master
> > driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > ---
> > Changes since v3:
> >   - Description  updated typo "Fixes:"
> >
> >  drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > b/drivers/i3c/master/svc-i3c-master.c
> > index d6057d8c7dec..98c4d2e5cd8d 100644
> > --- a/drivers/i3c/master/svc-i3c-master.c
> > +++ b/drivers/i3c/master/svc-i3c-master.c
> > @@ -534,8 +534,11 @@ static void svc_i3c_master_ibi_work(struct
> work_struct *work)
> >  	switch (ibitype) {
> >  	case SVC_I3C_MSTATUS_IBITYPE_IBI:
> >  		if (dev) {
> > -			i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> > -			master->ibi.tbq_slot = NULL;
> > +			data = i3c_dev_get_master_data(dev);
> > +			if (master->ibi.slots[data->ibi]) {
> > +				i3c_master_queue_ibi(dev, master-
> >ibi.tbq_slot);
> > +				master->ibi.tbq_slot = NULL;
> > +			}
> 
> You still not reply previous discussion:
> 
> https://lore.kernel.org/linux-i3c/Z8sOKZSjHeeP2mY5@lizhi-Precision-Tower-
> 5810/T/#mfd02d6ddca0a4b57bc823dcbfa7571c564800417
> 
[Manjunatha Venkatesh] : In the last mail answered to this question.

> This is not issue only at svc driver, which should be common problem for
> other master controller drivers
> 
[Manjunatha Venkatesh] :Yes, you are right.
One of my project I3C interface is required, where we have used IMX board as reference platform.
As part of boot sequence we come across this issue and tried to fix that particular controller driver.

What is your conclusion on this? Is it not ok to take patch for SVC alone?
 
> Frank
> 
> >  		}
> >  		svc_i3c_master_emit_stop(master);
> >  		break;
> > --
> > 2.46.1
> >

Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Frank Li]

On Thu, Mar 13, 2025 at 05:15:42AM +0000, Manjunatha Venkatesh wrote:
>
>
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: Wednesday, March 12, 2025 11:59 PM
> > To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> > alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> > kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> > Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at
> > svc_i3c_master_ibi_work()
> >
> > On Wed, Mar 12, 2025 at 07:23:56PM +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.
> > > 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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C master
> > > driver")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > > ---
> > > Changes since v3:
> > >   - Description  updated typo "Fixes:"
> > >
> > >  drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > > b/drivers/i3c/master/svc-i3c-master.c
> > > index d6057d8c7dec..98c4d2e5cd8d 100644
> > > --- a/drivers/i3c/master/svc-i3c-master.c
> > > +++ b/drivers/i3c/master/svc-i3c-master.c
> > > @@ -534,8 +534,11 @@ static void svc_i3c_master_ibi_work(struct
> > work_struct *work)
> > >  	switch (ibitype) {
> > >  	case SVC_I3C_MSTATUS_IBITYPE_IBI:
> > >  		if (dev) {
> > > -			i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> > > -			master->ibi.tbq_slot = NULL;
> > > +			data = i3c_dev_get_master_data(dev);
> > > +			if (master->ibi.slots[data->ibi]) {
> > > +				i3c_master_queue_ibi(dev, master-
> > >ibi.tbq_slot);
> > > +				master->ibi.tbq_slot = NULL;
> > > +			}
> >
> > You still not reply previous discussion:
> >
> > https://lore.kernel.org/linux-i3c/Z8sOKZSjHeeP2mY5@lizhi-Precision-Tower-
> > 5810/T/#mfd02d6ddca0a4b57bc823dcbfa7571c564800417
> >
> [Manjunatha Venkatesh] : In the last mail answered to this question.
>
> > This is not issue only at svc driver, which should be common problem for
> > other master controller drivers
> >
> [Manjunatha Venkatesh] :Yes, you are right.
> One of my project I3C interface is required, where we have used IMX board as reference platform.
> As part of boot sequence we come across this issue and tried to fix that particular controller driver.
>
> What is your conclusion on this? Is it not ok to take patch for SVC alone?

I perfer fix at common framwork to avoid every driver copy the similar
logic code.

Frank

>
> > Frank
> >
> > >  		}
> > >  		svc_i3c_master_emit_stop(master);
> > >  		break;
> > > --
> > > 2.46.1
> > >
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Manjunatha Venkatesh]

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Monday, March 17, 2025 6:57 PM
> To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at
> svc_i3c_master_ibi_work()
> 
> On Thu, Mar 13, 2025 at 05:15:42AM +0000, Manjunatha Venkatesh wrote:
> >
> >
> > > -----Original Message-----
> > > From: Frank Li <frank.li@nxp.com>
> > > Sent: Wednesday, March 12, 2025 11:59 PM
> > > To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > > Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> > > alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> > > kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> > > Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable
> > > memory at
> > > svc_i3c_master_ibi_work()
> > >
> > > On Wed, Mar 12, 2025 at 07:23:56PM +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.
> > > > 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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C master
> > > > driver")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Manjunatha Venkatesh
> <manjunatha.venkatesh@nxp.com>
> > > > ---
> > > > Changes since v3:
> > > >   - Description  updated typo "Fixes:"
> > > >
> > > >  drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > > > b/drivers/i3c/master/svc-i3c-master.c
> > > > index d6057d8c7dec..98c4d2e5cd8d 100644
> > > > --- a/drivers/i3c/master/svc-i3c-master.c
> > > > +++ b/drivers/i3c/master/svc-i3c-master.c
> > > > @@ -534,8 +534,11 @@ static void svc_i3c_master_ibi_work(struct
> > > work_struct *work)
> > > >  	switch (ibitype) {
> > > >  	case SVC_I3C_MSTATUS_IBITYPE_IBI:
> > > >  		if (dev) {
> > > > -			i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> > > > -			master->ibi.tbq_slot = NULL;
> > > > +			data = i3c_dev_get_master_data(dev);
> > > > +			if (master->ibi.slots[data->ibi]) {
> > > > +				i3c_master_queue_ibi(dev, master-
> > > >ibi.tbq_slot);
> > > > +				master->ibi.tbq_slot = NULL;
> > > > +			}
> > >
> > > You still not reply previous discussion:
> > >
> > > https://lore.kernel.org/linux-i3c/Z8sOKZSjHeeP2mY5@lizhi-Precision-T
> > > ower-
> > > 5810/T/#mfd02d6ddca0a4b57bc823dcbfa7571c564800417
> > >
> > [Manjunatha Venkatesh] : In the last mail answered to this question.
> >
> > > This is not issue only at svc driver, which should be common problem
> > > for other master controller drivers
> > >
> > [Manjunatha Venkatesh] :Yes, you are right.
> > One of my project I3C interface is required, where we have used IMX board
> as reference platform.
> > As part of boot sequence we come across this issue and tried to fix that
> particular controller driver.
> >
> > What is your conclusion on this? Is it not ok to take patch for SVC alone?
> 
> I perfer fix at common framwork to avoid every driver copy the similar logic
> code.
> 
[Manjunatha Venkatesh] : As per your suggestion tried the below patch at common framework api i3c_master_queue_ibi()
 and looks working fine, didn't see any crash issue.
if (!dev->ibi || !slot) {
             dev_warning("...");
             return;
}
Will commit this change in next patch submission.

> Frank
> 
> >
> > > Frank
> > >
> > > >  		}
> > > >  		svc_i3c_master_emit_stop(master);
> > > >  		break;
> > > > --
> > > > 2.46.1
> > > >
> >
> > --
> > linux-i3c mailing list
> > linux-i3c@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Alexandre Belloni]

On 17/03/2025 15:46:52+0000, Manjunatha Venkatesh wrote:
> 
> 
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: Monday, March 17, 2025 6:57 PM
> > To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> > alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> > kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> > Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at
> > svc_i3c_master_ibi_work()
> > 
> > On Thu, Mar 13, 2025 at 05:15:42AM +0000, Manjunatha Venkatesh wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Frank Li <frank.li@nxp.com>
> > > > Sent: Wednesday, March 12, 2025 11:59 PM
> > > > To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > > > Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> > > > alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> > > > kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> > > > Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable
> > > > memory at
> > > > svc_i3c_master_ibi_work()
> > > >
> > > > On Wed, Mar 12, 2025 at 07:23:56PM +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.
> > > > > 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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C master
> > > > > driver")
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Manjunatha Venkatesh
> > <manjunatha.venkatesh@nxp.com>
> > > > > ---
> > > > > Changes since v3:
> > > > >   - Description  updated typo "Fixes:"
> > > > >
> > > > >  drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> > > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > > > > b/drivers/i3c/master/svc-i3c-master.c
> > > > > index d6057d8c7dec..98c4d2e5cd8d 100644
> > > > > --- a/drivers/i3c/master/svc-i3c-master.c
> > > > > +++ b/drivers/i3c/master/svc-i3c-master.c
> > > > > @@ -534,8 +534,11 @@ static void svc_i3c_master_ibi_work(struct
> > > > work_struct *work)
> > > > >  	switch (ibitype) {
> > > > >  	case SVC_I3C_MSTATUS_IBITYPE_IBI:
> > > > >  		if (dev) {
> > > > > -			i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> > > > > -			master->ibi.tbq_slot = NULL;
> > > > > +			data = i3c_dev_get_master_data(dev);
> > > > > +			if (master->ibi.slots[data->ibi]) {
> > > > > +				i3c_master_queue_ibi(dev, master-
> > > > >ibi.tbq_slot);
> > > > > +				master->ibi.tbq_slot = NULL;
> > > > > +			}
> > > >
> > > > You still not reply previous discussion:
> > > >
> > > > https://lore.kernel.org/linux-i3c/Z8sOKZSjHeeP2mY5@lizhi-Precision-T
> > > > ower-
> > > > 5810/T/#mfd02d6ddca0a4b57bc823dcbfa7571c564800417
> > > >
> > > [Manjunatha Venkatesh] : In the last mail answered to this question.
> > >
> > > > This is not issue only at svc driver, which should be common problem
> > > > for other master controller drivers
> > > >
> > > [Manjunatha Venkatesh] :Yes, you are right.
> > > One of my project I3C interface is required, where we have used IMX board
> > as reference platform.
> > > As part of boot sequence we come across this issue and tried to fix that
> > particular controller driver.
> > >
> > > What is your conclusion on this? Is it not ok to take patch for SVC alone?
> > 
> > I perfer fix at common framwork to avoid every driver copy the similar logic
> > code.
> > 
> [Manjunatha Venkatesh] : As per your suggestion tried the below patch at common framework api i3c_master_queue_ibi()
>  and looks working fine, didn't see any crash issue.
> if (!dev->ibi || !slot) {
>              dev_warning("...");

Do we really need a warning, what would be the user action after seeing
it?
>              return;
> }
> Will commit this change in next patch submission.
> 
> > Frank
> > 
> > >
> > > > Frank
> > > >
> > > > >  		}
> > > > >  		svc_i3c_master_emit_stop(master);
> > > > >  		break;
> > > > > --
> > > > > 2.46.1
> > > > >
> > >
> > > --
> > > linux-i3c mailing list
> > > linux-i3c@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [EXT] Re: [PATCH v4] svc-i3c-master: Fix read from unreadable memory at svc_i3c_master_ibi_work()

[Manjunatha Venkatesh]

> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Monday, March 17, 2025 11:09 PM
> To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; miquel.raynal@bootlin.com;
> conor.culhane@silvaco.com; linux-i3c@lists.infradead.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> Subject: [EXT] Re: [PATCH v4] svc-i3c-master: Fix read from unreadable
> memory at svc_i3c_master_ibi_work()
> 
> 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 17/03/2025 15:46:52+0000, Manjunatha Venkatesh wrote:
> >
> >
> > > -----Original Message-----
> > > From: Frank Li <frank.li@nxp.com>
> > > Sent: Monday, March 17, 2025 6:57 PM
> > > To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > > Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> > > alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org; linux-
> > > kernel@vger.kernel.org; stable@vger.kernel.org; rvmanjumce@gmail.com
> > > Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable
> > > memory at
> > > svc_i3c_master_ibi_work()
> > >
> > > On Thu, Mar 13, 2025 at 05:15:42AM +0000, Manjunatha Venkatesh
> wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Frank Li <frank.li@nxp.com>
> > > > > Sent: Wednesday, March 12, 2025 11:59 PM
> > > > > To: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> > > > > Cc: miquel.raynal@bootlin.com; conor.culhane@silvaco.com;
> > > > > alexandre.belloni@bootlin.com; linux-i3c@lists.infradead.org;
> > > > > linux- kernel@vger.kernel.org; stable@vger.kernel.org;
> > > > > rvmanjumce@gmail.com
> > > > > Subject: Re: [PATCH v4] svc-i3c-master: Fix read from unreadable
> > > > > memory at
> > > > > svc_i3c_master_ibi_work()
> > > > >
> > > > > On Wed, Mar 12, 2025 at 07:23:56PM +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.
> > > > > > 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: dd3c52846d595 ("i3c: master: svc: Add Silvaco I3C
> > > > > > master
> > > > > > driver")
> > > > > > Cc: stable@vger.kernel.org
> > > > > > Signed-off-by: Manjunatha Venkatesh
> > > <manjunatha.venkatesh@nxp.com>
> > > > > > ---
> > > > > > Changes since v3:
> > > > > >   - Description  updated typo "Fixes:"
> > > > > >
> > > > > >  drivers/i3c/master/svc-i3c-master.c | 7 +++++--
> > > > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/i3c/master/svc-i3c-master.c
> > > > > > b/drivers/i3c/master/svc-i3c-master.c
> > > > > > index d6057d8c7dec..98c4d2e5cd8d 100644
> > > > > > --- a/drivers/i3c/master/svc-i3c-master.c
> > > > > > +++ b/drivers/i3c/master/svc-i3c-master.c
> > > > > > @@ -534,8 +534,11 @@ static void
> > > > > > svc_i3c_master_ibi_work(struct
> > > > > work_struct *work)
> > > > > >       switch (ibitype) {
> > > > > >       case SVC_I3C_MSTATUS_IBITYPE_IBI:
> > > > > >               if (dev) {
> > > > > > -                     i3c_master_queue_ibi(dev, master->ibi.tbq_slot);
> > > > > > -                     master->ibi.tbq_slot = NULL;
> > > > > > +                     data = i3c_dev_get_master_data(dev);
> > > > > > +                     if (master->ibi.slots[data->ibi]) {
> > > > > > +                             i3c_master_queue_ibi(dev,
> > > > > > + master-
> > > > > >ibi.tbq_slot);
> > > > > > +                             master->ibi.tbq_slot = NULL;
> > > > > > +                     }
> > > > >
> > > > > You still not reply previous discussion:
> > > > >
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Flore.kernel.org%2Flinux-i3c%2FZ8sOKZSjHeeP2mY5%40lizhi-Precisi
> > > > > on-
> T&data=05%7C02%7Cmanjunatha.venkatesh%40nxp.com%7Cf0ae17cf296
> > > > >
> 949cdd6c308dd657aa7e7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0
> > > > > %7C638778299668550575%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0
> eU1hcGkiOn
> > > > >
> RydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIj
> > > > >
> oyfQ%3D%3D%7C0%7C%7C%7C&sdata=3da5keE%2FKv9NbsltjDxywErXAjU1Al
> nR
> > > > > DXi5GQlMDJw%3D&reserved=0
> > > > > ower-
> > > > > 5810/T/#mfd02d6ddca0a4b57bc823dcbfa7571c564800417
> > > > >
> > > > [Manjunatha Venkatesh] : In the last mail answered to this question.
> > > >
> > > > > This is not issue only at svc driver, which should be common
> > > > > problem for other master controller drivers
> > > > >
> > > > [Manjunatha Venkatesh] :Yes, you are right.
> > > > One of my project I3C interface is required, where we have used
> > > > IMX board
> > > as reference platform.
> > > > As part of boot sequence we come across this issue and tried to
> > > > fix that
> > > particular controller driver.
> > > >
> > > > What is your conclusion on this? Is it not ok to take patch for SVC alone?
> > >
> > > I perfer fix at common framwork to avoid every driver copy the
> > > similar logic code.
> > >
> > [Manjunatha Venkatesh] : As per your suggestion tried the below patch
> > at common framework api i3c_master_queue_ibi()  and looks working fine,
> didn't see any crash issue.
> > if (!dev->ibi || !slot) {
> >              dev_warning("...");
> 
> Do we really need a warning, what would be the user action after seeing it?
[Manjunatha Venkatesh] : Warning message may not required here, we can just return it.
> >              return;
> > }
> > Will commit this change in next patch submission.
> >
> > > Frank
> > >
> > > >
> > > > > Frank
> > > > >
> > > > > >               }
> > > > > >               svc_i3c_master_emit_stop(master);
> > > > > >               break;
> > > > > > --
> > > > > > 2.46.1
> > > > > >
> > > >
> > > > --
> > > > linux-i3c mailing list
> > > > linux-i3c@lists.infradead.org
> > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fl
> > > > ists.infradead.org%2Fmailman%2Flistinfo%2Flinux-
> i3c&data=05%7C02%7
> > > >
> Cmanjunatha.venkatesh%40nxp.com%7Cf0ae17cf296949cdd6c308dd657aa7e
> 7
> > > > %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638778299668
> 572303%7
> > > >
> CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMD
> AwMCI
> > > >
> sIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sd
> a
> > > >
> ta=NupPno%2BfuqIUr8LiV5rrrO%2FJt%2B3AHol0nokRJjauWc8%3D&reserved
> =0
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel
> engineering
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootlin
> .com%2F&data=05%7C02%7Cmanjunatha.venkatesh%40nxp.com%7Cf0ae17c
> f296949cdd6c308dd657aa7e7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C638778299668585840%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0e
> U1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCI
> sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=lqapWA3WfBkuFZvpdF%2BJjVc
> ncKNjG%2FR7u1n6dK88TcY%3D&reserved=0