netdev - Re: [PATCH net-next] llc: fix netdevice reference leaks in llc_ui

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]

Date:   Thu, 24 Mar 2022 09:25:23 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        netdev <netdev@...r.kernel.org>,
        赵子轩 <beraphin@...il.com>,
        Stoyan Manolov <smanolov@...e.de>
Subject: Re: [PATCH net-next] llc: fix netdevice reference leaks in llc_ui_bind()

On Thu, Mar 24, 2022 at 7:38 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Wed, Mar 23, 2022 at 11:23 PM Dan Carpenter <dan.carpenter@...cle.com> wrote:
> >
> > On Tue, Mar 22, 2022 at 05:41:47PM -0700, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@...gle.com>
> > >
> > > Whenever llc_ui_bind() and/or llc_ui_autobind()
> > > took a reference on a netdevice but subsequently fail,
> > > they must properly release their reference
> > > or risk the infamous message from unregister_netdevice()
> > > at device dismantle.
> > >
> > > unregister_netdevice: waiting for eth0 to become free. Usage count = 3
> > >
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > > Reported-by: 赵子轩 <beraphin@...il.com>
> > > Reported-by: Stoyan Manolov <smanolov@...e.de>
> > > ---
> > >
> > > This can be applied on net tree, depending on how network maintainers
> > > plan to push the fix to Linus, this is obviously a stable candidate.
> >
> > This patch is fine, but it's that function is kind of ugly and difficult
> > for static analysis to parse.
>
> We usually do not mix bug fixes and code refactoring.
>
> Please feel free to send a refactor when net-next reopens in two weeks.
>
> Thanks.

I took another look at this code, and there might be an issue in llc_ui_bind(),
if the "goto out;" is taken _before_ we took a reference on a device.

We might now release the reference taken by a prior (and successful bind)

So it turns out another fix is needed.