linux-kernel - Re: [PATCH 5.10 09/38] 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220328093115.GB26815@amd>
Date:   Mon, 28 Mar 2022 11:31:16 +0200
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Pavel Machek <pavel@...x.de>, Jakub Kicinski <kuba@...nel.org>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Eric Dumazet <edumazet@...gle.com>,
        赵子轩 <beraphin@...il.com>,
        Stoyan Manolov <smanolov@...e.de>
Subject: Re: [PATCH 5.10 09/38] llc: fix netdevice reference leaks in
 llc_ui_bind()

Hi!

> > > > Should be in mainline on Thursday, LMK if we need to accelerate.
> > > > IDK if anyone enables LLC2.
> > > 
> > > I'll queue this up now, thanks.
> > 
> > As the changelog says, this needs b37a46683739, otherwise there will
> > be oops-es in even more cases.
> 
> If you look at the change, I think I already handled that issue.  If
> not, please let me know.

I did not notice you making changes there, but no, it is not correct
AFAICT.

# commit 163960a7de1333514c9352deb7c80c6b9fd9abf2
# Author: Eric Dumazet <edumazet@...gle.com>
# Date:   Thu Mar 24 20:58:27 2022 -0700

#    llc: only change llc->dev when bind() succeeds
...    
#     Make sure commit b37a46683739 ("netdevice: add the case if dev is NULL")
#     is already present in your trees.

Before b37a46683739, dev_put can't handle NULL.
    
+++ b/net/llc/af_llc.c
@@ -287,14 +288,14 @@ static int llc_ui_autobind(struct socket *sock, struct sockaddr_llc *addr)
...

-		llc->dev = dev_getfirstbyhwtype(&init_net, addr->sllc_arphrd);
-	if (!llc->dev)
+		dev = dev_getfirstbyhwtype(&init_net, addr->sllc_arphrd);
+	if (!dev)
 		goto out;
 	rc = -EUSERS;
 	llc->laddr.lsap = llc_ui_autoport();

One of several paths where we goto out with dev==NULL.

@@ -311,10 +317,7 @@ static int llc_ui_autobind(struct socket *sock, struct sockaddr_llc *addr)
 	sock_reset_flag(sk, SOCK_ZAPPED);
 	rc = 0;
 out:
-	if (rc) {
-		dev_put(llc->dev);
-		llc->dev = NULL;
-	}
+	dev_put(dev);
 	return rc;
 }


But dev_put can't handle NULL.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)