linux-kernel - Re: [syzbot] [kernel?] general protection fault in nfc_register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ZOyyn+5I5sYt/Udj@smile.fi.intel.com>
Date:   Mon, 28 Aug 2023 17:43:43 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     syzbot <syzbot+bdfb03b1ec8b342c12cb@...kaller.appspotmail.com>,
        hdanton@...a.com
Cc:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        rafael@...nel.org, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [kernel?] general protection fault in
 nfc_register_device

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