Re: [bug report] i3c: master: svc: Add Silvaco I3C master driver

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Dan,

Thanks for the report.

Dan Carpenter <dan.carpenter@oracle.com> wrote on Tue, 9 Feb 2021
13:57:28 +0300:

> Hello Miquel Raynal,
> 
> The patch dd3c52846d59: "i3c: master: svc: Add Silvaco I3C master
> driver" from Jan 21, 2021, leads to the following static checker
> warning:
> 
> 	drivers/i3c/master/svc-i3c-master.c:394 svc_i3c_master_ibi_work()
> 	error: uninitialized symbol 'dev'.
> 
> drivers/i3c/master/svc-i3c-master.c
>    364          status = readl(master->regs + SVC_I3C_MSTATUS);
>    365          ibitype = SVC_I3C_MSTATUS_IBITYPE(status);
>    366          ibiaddr = SVC_I3C_MSTATUS_IBIADDR(status);
>    367  
>    368          /* Handle the critical responses to IBI's */
>    369          switch (ibitype) {
>    370          case SVC_I3C_MSTATUS_IBITYPE_IBI:
>    371                  dev = svc_i3c_master_dev_from_addr(master, ibiaddr);
>    372                  if (!dev)
>    373                          svc_i3c_master_nack_ibi(master);
>    374                  else
>    375                          svc_i3c_master_handle_ibi(master, dev);
> 
> "dev" only valid on this path.
> 
>    376                  break;
>    377          case SVC_I3C_MSTATUS_IBITYPE_HOT_JOIN:
>    378                  svc_i3c_master_ack_ibi(master, false);
>    379                  break;
>    380          case SVC_I3C_MSTATUS_IBITYPE_MASTER_REQUEST:
>    381                  svc_i3c_master_nack_ibi(master);
>    382                  break;
>    383          default:
>    384                  break;
>    385          }
>    386  
>    387          /*
>    388           * If an error happened, we probably got interrupted and the exchange
>    389           * timedout. In this case we just drop everything, emit a stop and wait
>    390           * for the slave to interrupt again.
>    391           */
>    392          if (svc_i3c_master_error(master)) {
>    393                  if (master->ibi.tbq_slot) {
>    394                          data = i3c_dev_get_master_data(dev);
>                                                                ^^^
> Dereferenced here.

Only svc_i3c_master_handle_ibi() populates master->ibi.tbq_slot, so dev
will only be dereferenced if svc_i3c_master_handle_ibi() was executed,
which only happens if dev is valid.

I don't think there is a risk here, however if robots really are
unhappy we might add an extra sanity level by checking dev explicitly,
but this would be purely useless on a practical point of view.

Thanks,
Miquèl

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