Re: [PATCH 5.10 09/38] llc: fix netdevice reference leaks in llc_ui_bind()

Pavel Machek
2022-03-28T09:31:25Z

[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]

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@google.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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]