linux-kernel - Re: [PATCH] platform/x86: wmi: fix potential null pointer dereferences

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: <CAHp75Vdjq5S3gdk7iPEsbmAvxL1nVxxNSnTQ1O0KLxZ4uTH_tw@mail.gmail.com>
Date:   Mon, 28 Jan 2019 17:49:34 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Mattias Jacobsson <2pi@....nu>
Cc:     Darren Hart <dvhart@...radead.org>,
        Andy Shevchenko <andy@...radead.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: wmi: fix potential null pointer dereferences

On Mon, Jan 28, 2019 at 4:39 PM Mattias Jacobsson <2pi@....nu> wrote:
> On 2019-01-27, Andy Shevchenko wrote:
> > On Tue, Jan 22, 2019 at 10:04 PM Mattias Jacobsson <2pi@....nu> wrote:

> > > -       struct wmi_driver *wmi_driver =
> > > -               container_of(driver, struct wmi_driver, driver);
> >
> > AFAIU this is just a pointer arithmetics, no need to move it.
>
> That is my understanding too, it seamed backwards to do the NULL check
> afterwards, but we still have access to dev and driver. So why not...
>
> >
> > > -       struct wmi_block *wblock = dev_to_wblock(dev);
> >
> > > -       const struct wmi_device_id *id = wmi_driver->id_table;
> > > +       const struct wmi_device_id *id;
> > > +       struct wmi_block *wblock;
> > > +       struct wmi_driver *wmi_driver;
> > > +
> >
> > > +       if (dev == NULL || driver == NULL)
> > > +               return 0;
> >
> > On which circumstances this may ever happen?
>
> Nothing in particular. If there is a bug in the caller of this function,
> then that is when this will come into play. See my earlier mail to
> Darren too.

So, I suggest in these cases do not touch existing till the actual
problem will be discovered and proved.
I.o.w. touch only place which you have real problem, and describe this
issue in commit message.

-- 
With Best Regards,
Andy Shevchenko