| Message ID | 20250224031754.1001101-1-manjunatha.venkatesh@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | driver: svc-i3c-master: Kernel panic issue fixed | expand |
Subject should be "Fix read from unreadable memory at svc_i3c_master_ibi_work()" Or like below pattern: Do ... to fix ... On Mon, Feb 24, 2025 at 08:47:54AM +0530, Manjunatha Venkatesh wrote: > I3C Master driver svc-i3c-master Needn't this line. > > 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. Can you provide more detail information why/how this happen? which varible read cause this problem? Need fix tags and cc stable. Frank > > Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com> > --- > 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 > > > -- > linux-i3c mailing list > linux-i3c@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-i3c
Hi Frank, Please find my inline comments. Regards, Manju > -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: Friday, February 28, 2025 9:19 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; rvmanjumce@gmail.com > Subject: Re: [PATCH] driver: svc-i3c-master: Kernel panic issue fixed > > Subject should be "Fix read from unreadable memory at > svc_i3c_master_ibi_work()" > Or like below pattern: > Do ... to fix ... > [Manjunatha Venkatesh] Tittle updated as per the review comment. > On Mon, Feb 24, 2025 at 08:47:54AM +0530, Manjunatha Venkatesh wrote: > > I3C Master driver svc-i3c-master > > Needn't this line. > [Manjunatha Venkatesh] Removed this line and submitted the new patch > > > > 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. > > Can you provide more detail information why/how this happen? which > varible read cause this problem? > [Manjunatha Venkatesh] : 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. > Need fix tags and cc stable. > [Manjunatha Venkatesh] : fix tags and cc stable updated in the commit message. > Frank > > > > Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com> > > --- > > 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 > > > > > > -- > > linux-i3c mailing list > > linux-i3c@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-i3c
On Thu, Mar 06, 2025 at 08:31:29AM +0000, Manjunatha Venkatesh wrote: > Hi Frank, > > Please find my inline comments. > > Regards, > Manju Avoid top post for linux kernel mail list. > > > -----Original Message----- > > From: Frank Li <frank.li@nxp.com> > > Sent: Friday, February 28, 2025 9:19 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; rvmanjumce@gmail.com > > Subject: Re: [PATCH] driver: svc-i3c-master: Kernel panic issue fixed > > > > Subject should be "Fix read from unreadable memory at > > svc_i3c_master_ibi_work()" > > Or like below pattern: > > Do ... to fix ... > > > [Manjunatha Venkatesh] Tittle updated as per the review comment. > > On Mon, Feb 24, 2025 at 08:47:54AM +0530, Manjunatha Venkatesh wrote: > > > I3C Master driver svc-i3c-master > > > > Needn't this line. > > > [Manjunatha Venkatesh] Removed this line and submitted the new patch > > > > > > 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. > > > > Can you provide more detail information why/how this happen? which > > varible read cause this problem? > > > [Manjunatha Venkatesh] : 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. Write above reproduce steps to commit message. I think it is common problem for i3c framework. You'd better change i3c_master_queue_ibi() void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) { if (!slot) return; atomic_inc(&dev->ibi->pending_ibis); queue_work(dev->ibi->wq, &slot->work); } Frank > > > > Need fix tags and cc stable. > > > [Manjunatha Venkatesh] : fix tags and cc stable updated in the commit message. > > > Frank > > > > > > Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com> > > > --- > > > 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 > > > > > > > > > -- > > > linux-i3c mailing list > > > linux-i3c@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-i3c
> -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: Thursday, March 6, 2025 9:01 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; rvmanjumce@gmail.com > Subject: Re: [PATCH] driver: svc-i3c-master: Kernel panic issue fixed > > On Thu, Mar 06, 2025 at 08:31:29AM +0000, Manjunatha Venkatesh wrote: > > Hi Frank, > > > > Please find my inline comments. > > > > Regards, > > Manju > > Avoid top post for linux kernel mail list. > > > > > > -----Original Message----- > > > From: Frank Li <frank.li@nxp.com> > > > Sent: Friday, February 28, 2025 9:19 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; rvmanjumce@gmail.com > > > Subject: Re: [PATCH] driver: svc-i3c-master: Kernel panic issue > > > fixed > > > > > > Subject should be "Fix read from unreadable memory at > > > svc_i3c_master_ibi_work()" > > > Or like below pattern: > > > Do ... to fix ... > > > > > [Manjunatha Venkatesh] Tittle updated as per the review comment. > > > On Mon, Feb 24, 2025 at 08:47:54AM +0530, Manjunatha Venkatesh > wrote: > > > > I3C Master driver svc-i3c-master > > > > > > Needn't this line. > > > > > [Manjunatha Venkatesh] Removed this line and submitted the new patch > > > > > > > > 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. > > > > > > Can you provide more detail information why/how this happen? which > > > varible read cause this problem? > > > > > [Manjunatha Venkatesh] : 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. > > Write above reproduce steps to commit message. > [Manjunatha Venkatesh] : Next commit will update this description. > I think it is common problem for i3c framework. > > You'd better change i3c_master_queue_ibi() > > void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) > { > if (!slot) > return; > > atomic_inc(&dev->ibi->pending_ibis); > queue_work(dev->ibi->wq, &slot->work); } > [Manjunatha Venkatesh] : Actual issue we have faced with respect to 'dev->ibi' variable. Pointer variable 'slot' is with respect IBI received from slave device context and 'dev->ibi' is with respect to initialization of slave driver these 2 are asynchronous events. dev->ibi = ibi; is initialized at i3c_dev_request_ibi_locked(), this is invoked as part of slave driver IBI init sequence, this is what we want to check before processing the IBI request from the Slave device. master->ibi.slots[i] this is where all device ibi instance list are saved, so here our slave driver init IBI stored with respect dev->ibi index. So adding 'if (!slot)' will not help to resolve this issue. > Frank > > > > > > > Need fix tags and cc stable. > > > > > [Manjunatha Venkatesh] : fix tags and cc stable updated in the commit > message. > > [Manjunatha Venkatesh] : Last commit message missed to update fix tags and cc stable. Will update that information as part of next commit message. > > > Frank > > > > > > > > Signed-off-by: Manjunatha Venkatesh > <manjunatha.venkatesh@nxp.com> > > > > --- > > > > 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 > > > > > > > > > > > > -- > > > > linux-i3c mailing list > > > > linux-i3c@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-i3c
On Fri, Mar 07, 2025 at 12:34:53PM +0000, Manjunatha Venkatesh wrote: > > > > -----Original Message----- > > From: Frank Li <frank.li@nxp.com> > > Sent: Thursday, March 6, 2025 9:01 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; rvmanjumce@gmail.com > > Subject: Re: [PATCH] driver: svc-i3c-master: Kernel panic issue fixed > > > > On Thu, Mar 06, 2025 at 08:31:29AM +0000, Manjunatha Venkatesh wrote: > > > Hi Frank, > > > > > > Please find my inline comments. > > > > > > Regards, > > > Manju > > > > Avoid top post for linux kernel mail list. > > > > > > > > > -----Original Message----- > > > > From: Frank Li <frank.li@nxp.com> > > > > Sent: Friday, February 28, 2025 9:19 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; rvmanjumce@gmail.com > > > > Subject: Re: [PATCH] driver: svc-i3c-master: Kernel panic issue > > > > fixed > > > > > > > > Subject should be "Fix read from unreadable memory at > > > > svc_i3c_master_ibi_work()" > > > > Or like below pattern: > > > > Do ... to fix ... > > > > > > > [Manjunatha Venkatesh] Tittle updated as per the review comment. > > > > On Mon, Feb 24, 2025 at 08:47:54AM +0530, Manjunatha Venkatesh > > wrote: > > > > > I3C Master driver svc-i3c-master > > > > > > > > Needn't this line. > > > > > > > [Manjunatha Venkatesh] Removed this line and submitted the new patch > > > > > > > > > > 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. > > > > > > > > Can you provide more detail information why/how this happen? which > > > > varible read cause this problem? > > > > > > > [Manjunatha Venkatesh] : 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. > > > > Write above reproduce steps to commit message. > > > [Manjunatha Venkatesh] : Next commit will update this description. > > I think it is common problem for i3c framework. > > > > You'd better change i3c_master_queue_ibi() > > > > void i3c_master_queue_ibi(struct i3c_dev_desc *dev, struct i3c_ibi_slot *slot) > > { > > if (!slot) > > return; > > > > atomic_inc(&dev->ibi->pending_ibis); > > queue_work(dev->ibi->wq, &slot->work); } > > > [Manjunatha Venkatesh] : Actual issue we have faced with respect to 'dev->ibi' variable. How about if (!dev->ibi || !slot) { dev_warning("..."); return; } Frank > Pointer variable 'slot' is with respect IBI received from slave device context and 'dev->ibi' is with respect to initialization of slave driver these 2 are asynchronous events. > dev->ibi = ibi; is initialized at i3c_dev_request_ibi_locked(), this is invoked as part of slave driver IBI init sequence, this is what we want to check before processing the IBI request from the Slave device. > master->ibi.slots[i] this is where all device ibi instance list are saved, so here our slave driver init IBI stored with respect dev->ibi index. > So adding 'if (!slot)' will not help to resolve this issue. Please wrap around at 80 char to read easily. > > > Frank > > > > > > > > > > Need fix tags and cc stable. > > > > > > > [Manjunatha Venkatesh] : fix tags and cc stable updated in the commit > > message. > > > > [Manjunatha Venkatesh] : Last commit message missed to update fix tags and cc stable. > Will update that information as part of next commit message. > > > > Frank > > > > > > > > > > Signed-off-by: Manjunatha Venkatesh > > <manjunatha.venkatesh@nxp.com> > > > > > --- > > > > > 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 > > > > > > > > > > > > > > > -- > > > > > linux-i3c mailing list > > > > > linux-i3c@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-i3c
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;
I3C Master driver svc-i3c-master 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. Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com> --- drivers/i3c/master/svc-i3c-master.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)