Skip to content
Snippets Groups Projects
Commit b694ae58 authored by Eric Dumazet's avatar Eric Dumazet Committed by Yongqiang Liu
Browse files

llc: only change llc->dev when bind() succeeds

stable inclusion
from stable-v4.19.237
commit c106f9aa6cd8913af9188ad361899ae696b5de37
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/I51YBN
CVE: CVE-2022-28356

-------------------------------------------------

commit 2d327a79ee176930dc72c131a970c891d367c1dc upstream.

My latest patch, attempting to fix the refcount leak in a minimal
way turned out to add a new bug.

Whenever the bind operation fails before we attempt to grab
a reference count on a device, we might release the device refcount
of a prior successful bind() operation.

syzbot was not happy about this [1].

Note to stable teams:

Make sure commit b37a46683739 ("netdevice: add the case if dev is NULL")
is already present in your trees.

[1]
general protection fault, probably for non-canonical address 0xdffffc0000000070: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000380-0x0000000000000387]
CPU: 1 PID: 3590 Comm: syz-executor361 Tainted: G    ...
parent ca0c52d1
No related branches found
No related tags found
No related merge requests found
......@@ -268,6 +268,7 @@ static int llc_ui_autobind(struct socket *sock, struct sockaddr_llc *addr)
{
struct sock *sk = sock->sk;
struct llc_sock *llc = llc_sk(sk);
struct net_device *dev = NULL;
struct llc_sap *sap;
int rc = -EINVAL;
......@@ -279,14 +280,14 @@ static int llc_ui_autobind(struct socket *sock, struct sockaddr_llc *addr)
goto out;
rc = -ENODEV;
if (sk->sk_bound_dev_if) {
llc->dev = dev_get_by_index(&init_net, sk->sk_bound_dev_if);
if (llc->dev && addr->sllc_arphrd != llc->dev->type) {
dev_put(llc->dev);
llc->dev = NULL;
dev = dev_get_by_index(&init_net, sk->sk_bound_dev_if);
if (dev && addr->sllc_arphrd != dev->type) {
dev_put(dev);
dev = NULL;
}
} else
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();
......@@ -296,6 +297,11 @@ static int llc_ui_autobind(struct socket *sock, struct sockaddr_llc *addr)
sap = llc_sap_open(llc->laddr.lsap, NULL);
if (!sap)
goto out;
/* Note: We do not expect errors from this point. */
llc->dev = dev;
dev = NULL;
memcpy(llc->laddr.mac, llc->dev->dev_addr, IFHWADDRLEN);
memcpy(&llc->addr, addr, sizeof(llc->addr));
/* assign new connection to its SAP */
......@@ -303,10 +309,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;
}
......@@ -329,6 +332,7 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
struct sockaddr_llc *addr = (struct sockaddr_llc *)uaddr;
struct sock *sk = sock->sk;
struct llc_sock *llc = llc_sk(sk);
struct net_device *dev = NULL;
struct llc_sap *sap;
int rc = -EINVAL;
......@@ -344,25 +348,26 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
rc = -ENODEV;
rcu_read_lock();
if (sk->sk_bound_dev_if) {
llc->dev = dev_get_by_index_rcu(&init_net, sk->sk_bound_dev_if);
if (llc->dev) {
dev = dev_get_by_index_rcu(&init_net, sk->sk_bound_dev_if);
if (dev) {
if (is_zero_ether_addr(addr->sllc_mac))
memcpy(addr->sllc_mac, llc->dev->dev_addr,
memcpy(addr->sllc_mac, dev->dev_addr,
IFHWADDRLEN);
if (addr->sllc_arphrd != llc->dev->type ||
if (addr->sllc_arphrd != dev->type ||
!ether_addr_equal(addr->sllc_mac,
llc->dev->dev_addr)) {
dev->dev_addr)) {
rc = -EINVAL;
llc->dev = NULL;
dev = NULL;
}
}
} else
llc->dev = dev_getbyhwaddr_rcu(&init_net, addr->sllc_arphrd,
} else {
dev = dev_getbyhwaddr_rcu(&init_net, addr->sllc_arphrd,
addr->sllc_mac);
if (llc->dev)
dev_hold(llc->dev);
}
if (dev)
dev_hold(dev);
rcu_read_unlock();
if (!llc->dev)
if (!dev)
goto out;
if (!addr->sllc_sap) {
rc = -EUSERS;
......@@ -395,6 +400,11 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
goto out_put;
}
}
/* Note: We do not expect errors from this point. */
llc->dev = dev;
dev = NULL;
llc->laddr.lsap = addr->sllc_sap;
memcpy(llc->laddr.mac, addr->sllc_mac, IFHWADDRLEN);
memcpy(&llc->addr, addr, sizeof(llc->addr));
......@@ -405,10 +415,7 @@ static int llc_ui_bind(struct socket *sock, struct sockaddr *uaddr, int addrlen)
out_put:
llc_sap_put(sap);
out:
if (rc) {
dev_put(llc->dev);
llc->dev = NULL;
}
dev_put(dev);
release_sock(sk);
return rc;
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment