Re: [syzbot] [kernel?] general protection fault in nfc_register_device

[Andy Shevchenko]

On Mon, Aug 28, 2023 at 03:42:28PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 28, 2023 at 02:35:22PM +0300, Andy Shevchenko wrote:
> > On Mon, Aug 28, 2023 at 02:53:37AM -0700, syzbot wrote:
> > > Hello,
> > > 
> > > syzbot found the following issue on:
> > 
> > Thanks! Will work on it ASAP.
> 
> So, can somebody from syzbot explain me what this is?
> 
> My readings as follows:
> 1) syzbot inserts fault injection into
> 
>         dev_set_name(&dev->dev, "nfc%d", dev->idx);
> 
> 2) that becomes a NULL in the corresponding kobj->name, correct?
> 3) which leads to the device_add() to exercise the paths that check for name
>    being NULL, i.e.
> 
>      if (dev->init_name) {
>         error = dev_set_name(dev, "%s", dev->init_name);
>         dev->init_name = NULL;
>      }
> 
>      if (dev_name(dev))
>          error = 0;
>      /* subsystems can specify simple device enumeration */
>      else if (dev->bus && dev->bus->dev_name)
>          error = dev_set_name(dev, "%s%u", dev->bus->dev_name, dev->id);
>      if (error)
>          goto name_error;
> 
>    so, either we have init_name or bus->name defined, but this seems not
>    the case; anyway in both cases the dev_set_name() may fail in strchr()
>    if and only if the fmt is NULL, which is not, as above calls have it
>    hard coded literals.
> 
> What's going on here?
> 
> (The error check for dev_set_name() in nfc_allocate_device() probably fixes
>  this, but it must not affect the code execution, right?)
> 
> P.S.
> Of course the test patch from hdanton@ doesn't fix it, the error is checked in
> the code later on.

Looking at the second patch from @hdanton I understand what may have happened.
The value of "error" has been rewritten to 0 and dev_name() the new code
doesn't trigger the bail out.

I'll send the patch soon.

-- 
With Best Regards,
Andy Shevchenko