linux-kernel - Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req

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]

Date:   Thu, 23 Sep 2021 13:07:17 +0200
From:   Greg KH <greg@...ah.com>
To:     Pavel Skripkin <paskripkin@...il.com>
Cc:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>,
        Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        David Laight <david.Laight@...lab.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Martin Kaiser <martin@...ser.cx>
Subject: Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex

On Thu, Sep 23, 2021 at 01:12:53PM +0300, Pavel Skripkin wrote:
> On 9/23/21 11:47, Pavel Skripkin wrote:
> > On 9/22/21 16:21, Pavel Skripkin wrote:
> > > On 9/21/21 21:18, Fabio M. De Francesco wrote:
> > > > From: Pavel Skripkin <paskripkin@...il.com>
> > > > 
> > > > This mutex was used to protect shared buffer for USB requests. Since
> > > > buffer was removed in previous patch we can remove this mutex as well.
> > > > 
> > > > Furthermore, because it was used to serialize the calls to the Core USB
> > > > API, we thoroughly tested the enabling of concurrent firing of USB requests
> > > > without the mutex and found no problems of any kind in common use cases.
> > > > 
> > > > Co-developed-by: Fabio M. De Francesco <fmdefrancesco@...il.com>
> > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.com>
> > > > Signed-off-by: Pavel Skripkin <paskripkin@...il.com>
> > > 
> > > Hi, Greg!
> > > 
> > > If all is OK with previous 15 patches, please, do not take this one, it
> > >    causes problems with connection... :)
> > > 
> > > I don't understand what went wrong after v8, but anyway, this one should
> > > not be applied for now, since it's broken
> > > 
> > > 
> > > Thank you
> > > 
> > > 
> > 
> > 
> > Just to be clear: previous 15 patches _are_ tested and do not cause any
> > misbehavior or bugs.
> > 
> > I guess, the stack buffer maybe the problem here, since it's the only
> > change on this side since v8. I didn't have a chance to take a closer
> > look, but I will do it on weekends, I hope :)
> > 
> 
> Oh, I found the problem by just looking at the code with clear mind:
> 
> > -free_dvobj:
> > -	if (status != _SUCCESS && pdvobjpriv) {
> > +	if (pdvobjpriv) {
> >  		usb_set_intfdata(usb_intf, NULL);
> >  		kfree(pdvobjpriv);
> >  		pdvobjpriv = NULL;
> 
> This if should be deleted completely, because we don't want to fail on every
> probe :)
> 
> Sorry for noise... :(
> 
> Greg, can you take first 15 patches, if they look good and then I will send
> fixed version of 16? AFAIU, you are ok with taking part of the series

Please fix up and resend the whole series as our tools work best by
taking the whole thing.

That way I "know" you tested them all :)

thanks,

greg k-h