netdev - Re: [PATCH 2/7] ibmvnic: update reset function prototypes

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: <20210111195759.GA178503@us.ibm.com>
Date:   Mon, 11 Jan 2021 11:57:59 -0800
From:   Sukadev Bhattiprolu <sukadev@...ux.ibm.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, Dany Madden <drt@...ux.ibm.com>,
        Lijun Pan <ljp@...ux.ibm.com>
Subject: Re: [PATCH 2/7] ibmvnic: update reset function prototypes

Jakub Kicinski [kuba@...nel.org] wrote:
> On Sun, 10 Jan 2021 19:12:21 -0800 Sukadev Bhattiprolu wrote:
> > Jakub Kicinski [kuba@...nel.org] wrote:
> > > On Thu,  7 Jan 2021 23:12:31 -0800 Sukadev Bhattiprolu wrote:  
> > > > The reset functions need just the 'reset reason' parameter and not
> > > > the ibmvnic_rwi list element. Update the functions so we can simplify
> > > > the handling of the ->rwi_list in a follow-on patch.
> > > > 
> > > > Fixes: 2770a7984db5 ("ibmvnic: Introduce hard reset recovery")
> > > >   
> > > 
> > > No empty lines after Fixes tags, please. They should also not be
> > > wrapped.  
> > 
> > Ah ok, will fix.
> > >   
> > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@...ux.ibm.com>  
> > > 
> > > Are these patches for net or net-next? It looks like they are fixing
> > > races, but at the same time they are rather large. Can you please
> > > produce minimal fixes, e.g. patch 3 should just fix the existing leaks
> > > rather than refactor the code to not do allocations. 130+ LoC is a lot
> > > for a fix.  
> > 
> > This is a set of bug fixes, but yes a bit large. Should I submit to
> > net-next instead?
> 
> I'd rather you tried to address the problems with minimal patches, then
> you can refactor the code in net-next.

I had thought about that but fixing the leaks and then throwing away the
code did not seem very useful. The diff stat shows 78 insertions and 59
deletions. The bulk of the new code, about 70 lines including comments,
is just the fairly straightforward helper functions:

	- get_pending_reset()
	- add_pending_reset() 

Fixing the leak in the existing code would not reduce the size of these
helpers. We are now using a simpler approach with no allocations, so no
leaks.

Sukadev