linux-kernel - RE: [PATCH v3 5/6] staging: r8188eu: add error handling of rtw

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <64112dfd8e8b472ab78ca8b4837dc9e9@AcuMS.aculab.com>
Date:   Thu, 26 Aug 2021 10:22:25 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Pavel Skripkin' <paskripkin@...il.com>
CC:     "Larry.Finger@...inger.net" <Larry.Finger@...inger.net>,
        "phil@...lpotter.co.uk" <phil@...lpotter.co.uk>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "straube.linux@...il.com" <straube.linux@...il.com>,
        "fmdefrancesco@...il.com" <fmdefrancesco@...il.com>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3 5/6] staging: r8188eu: add error handling of rtw_read32

From: Pavel Skripkin
> Sent: 26 August 2021 10:28
> 
> On 8/26/21 12:22 PM, Pavel Skripkin wrote:
> > On Thu, 26 Aug 2021 08:51:23 +0000
> > David Laight <David.Laight@...LAB.COM> wrote:
...
> >> ...
> >> > -static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
> >> > +static int usb_read32(struct intf_hdl *pintfhdl, u32 addr, u32
> >> > *data) {
> >> >  	u8 requesttype;
> >> >  	u16 wvalue;
> >> >  	u16 len;
> >> > -	__le32 data;
> >> > +	int res;
> >> > +	__le32 tmp;
> >> > +
> >> > +	if (WARN_ON(unlikely(!data)))
> >> > +		return -EINVAL;
> >> >
> >>
> >> Kill the NULL check - it is a silly coding error.
> >> An OOPS is just as easy to debug.
> >>
> >
> >
> > I don't think that one single driver should kill the whole system. It's
> > 100% an error, but kernel can recover from it (for example disconnect
> > the driver, since it's broken).
> >
> >
> > AFIAK, Greg and Linus do not like BUG_ONs in recoverable state...
> > Correct me, if I am wrong
> >
> Oops, I thought about BUG_ON() instead of WARN_ON(), sorry for
> confusion. My point is "we should not let the box boom".


There is no point checking for NULL that just can't happen.
In this case all the callers will pass the address of a local.
There really is no point checking.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)