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@xxxxxxx>
> Sent: Wednesday, March 12, 2025 11:59 PM
> To: Manjunatha Venkatesh <manjunatha.venkatesh@xxxxxxx>
> Cc: miquel.raynal@xxxxxxxxxxx; conor.culhane@xxxxxxxxxxx;
> alexandre.belloni@xxxxxxxxxxx; linux-i3c@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx; rvmanjumce@xxxxxxxxx
> 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@xxxxxxx>
> > ---
> > 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
> >