nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

can: raw: fix receiver memory leak

Browse source 
Got kmemleak errors with the following ltp can_filter testcase:

for ((i=1; i<=100; i++))
do
        ./can_filter &
        sleep 0.1
done

==============================================================
[<00000000db4a4943>] can_rx_register+0x147/0x360 [can]
[<00000000a289549d>] raw_setsockopt+0x5ef/0x853 [can_raw]
[<000000006d3d9ebd>] __sys_setsockopt+0x173/0x2c0
[<00000000407dbfec>] __x64_sys_setsockopt+0x61/0x70
[<00000000fd468496>] do_syscall_64+0x33/0x40
[<00000000b7e47d51>] entry_SYSCALL_64_after_hwframe+0x61/0xc6

It's a bug in the concurrent scenario of unregister_netdevice_many()
and raw_release() as following:

             cpu0                                        cpu1
unregister_netdevice_many(can_dev)
  unlist_netdevice(can_dev) // dev_get_by_index() return NULL after this
  net_set_todo(can_dev)
						raw_release(can_socket)
						  dev = dev_get_by_index(, ro->ifindex); // dev == NULL
						  if (dev) { // receivers in dev_rcv_lists not free because dev is NULL
						    raw_disable_allfilters(, dev, );
						    dev_put(dev);
						  }
						  ...
						  ro->bound = 0;
						  ...

call_netdevice_notifiers(NETDEV_UNREGISTER, )
  raw_notify(, NETDEV_UNREGISTER, )
    if (ro->bound) // invalid because ro->bound has been set 0
      raw_disable_allfilters(, dev, ); // receivers in dev_rcv_lists will never be freed

Add a net_device pointer member in struct raw_sock to record bound
can_dev, and use rtnl_lock to serialize raw_socket members between
raw_bind(), raw_release(), raw_setsockopt() and raw_notify(). Use
ro->dev to decide whether to free receivers in dev_rcv_lists.

Fixes: 8d0caedb75 ("can: bcm/raw/isotp: use per module netdevice notifier")
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
Link: https://lore.kernel.org/all/20230711011737.1969582-1-william.xuanziyang@huawei.com
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This commit is contained in:
Ziyang Xuan 2023-07-11 09:17:37 +08:00 committed by Marc Kleine-Budde
parent 0dd1805fe4
commit ee8b94c851
1 changed files with 24 additions and 33 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

57
net/can/raw.c
View file

@ -84,6 +84,7 @@ struct raw_sock {
struct sock sk; struct sock sk;
int bound; int bound;
int ifindex; int ifindex;
struct net_device *dev;
struct list_head notifier; struct list_head notifier;
int loopback; int loopback;
int recv_own_msgs; int recv_own_msgs;
@ -277,7 +278,7 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg,
if (!net_eq(dev_net(dev), sock_net(sk))) if (!net_eq(dev_net(dev), sock_net(sk)))
return; return;
if (ro->ifindex != dev->ifindex) if (ro->dev != dev)
return; return;
switch (msg) { switch (msg) {
@ -292,6 +293,7 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg,
ro->ifindex = 0; ro->ifindex = 0;
ro->bound = 0; ro->bound = 0;
ro->dev = NULL;
ro->count = 0; ro->count = 0;
release_sock(sk); release_sock(sk);
@ -337,6 +339,7 @@ static int raw_init(struct sock *sk)
ro->bound = 0; ro->bound = 0;
ro->ifindex = 0; ro->ifindex = 0;
ro->dev = NULL;
/* set default filter to single entry dfilter */ /* set default filter to single entry dfilter */
ro->dfilter.can_id = 0; ro->dfilter.can_id = 0;
@ -385,19 +388,13 @@ static int raw_release(struct socket *sock)
lock_sock(sk); lock_sock(sk);
rtnl_lock();
/* remove current filters & unregister */ /* remove current filters & unregister */
if (ro->bound) { if (ro->bound) {
if (ro->ifindex) { if (ro->dev)
struct net_device *dev; raw_disable_allfilters(dev_net(ro->dev), ro->dev, sk);
else
dev = dev_get_by_index(sock_net(sk), ro->ifindex);
if (dev) {
raw_disable_allfilters(dev_net(dev), dev, sk);
dev_put(dev);
}
} else {
raw_disable_allfilters(sock_net(sk), NULL, sk); raw_disable_allfilters(sock_net(sk), NULL, sk);
}
} }
if (ro->count > 1) if (ro->count > 1)
@ -405,8 +402,10 @@ static int raw_release(struct socket *sock)
ro->ifindex = 0; ro->ifindex = 0;
ro->bound = 0; ro->bound = 0;
ro->dev = NULL;
ro->count = 0; ro->count = 0;
free_percpu(ro->uniq); free_percpu(ro->uniq);
rtnl_unlock();
sock_orphan(sk); sock_orphan(sk);
sock->sk = NULL; sock->sk = NULL;
@ -422,6 +421,7 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len)
struct sockaddr_can *addr = (struct sockaddr_can *)uaddr; struct sockaddr_can *addr = (struct sockaddr_can *)uaddr;
struct sock *sk = sock->sk; struct sock *sk = sock->sk;
struct raw_sock *ro = raw_sk(sk); struct raw_sock *ro = raw_sk(sk);
struct net_device *dev = NULL;
int ifindex; int ifindex;
int err = 0; int err = 0;
int notify_enetdown = 0; int notify_enetdown = 0;
@ -431,14 +431,13 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len)
if (addr->can_family != AF_CAN) if (addr->can_family != AF_CAN)
return -EINVAL; return -EINVAL;
rtnl_lock();
lock_sock(sk); lock_sock(sk);
if (ro->bound && addr->can_ifindex == ro->ifindex) if (ro->bound && addr->can_ifindex == ro->ifindex)
goto out; goto out;
if (addr->can_ifindex) { if (addr->can_ifindex) {
struct net_device *dev;
dev = dev_get_by_index(sock_net(sk), addr->can_ifindex); dev = dev_get_by_index(sock_net(sk), addr->can_ifindex);
if (!dev) { if (!dev) {
err = -ENODEV; err = -ENODEV;
@ -467,26 +466,20 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len)
if (!err) { if (!err) {
if (ro->bound) { if (ro->bound) {
/* unregister old filters */ /* unregister old filters */
if (ro->ifindex) { if (ro->dev)
struct net_device *dev; raw_disable_allfilters(dev_net(ro->dev),
ro->dev, sk);
dev = dev_get_by_index(sock_net(sk), else
ro->ifindex);
if (dev) {
raw_disable_allfilters(dev_net(dev),
dev, sk);
dev_put(dev);
}
} else {
raw_disable_allfilters(sock_net(sk), NULL, sk); raw_disable_allfilters(sock_net(sk), NULL, sk);
}
} }
ro->ifindex = ifindex; ro->ifindex = ifindex;
ro->bound = 1; ro->bound = 1;
ro->dev = dev;
} }
out: out:
release_sock(sk); release_sock(sk);
rtnl_unlock();
if (notify_enetdown) { if (notify_enetdown) {
sk->sk_err = ENETDOWN; sk->sk_err = ENETDOWN;
@ -553,9 +546,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
rtnl_lock(); rtnl_lock();
lock_sock(sk); lock_sock(sk);
if (ro->bound && ro->ifindex) { dev = ro->dev;
dev = dev_get_by_index(sock_net(sk), ro->ifindex); if (ro->bound && dev) {
if (!dev) { if (dev->reg_state != NETREG_REGISTERED) {
if (count > 1) if (count > 1)
kfree(filter); kfree(filter);
err = -ENODEV; err = -ENODEV;
@ -596,7 +589,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
ro->count = count; ro->count = count;
out_fil: out_fil:
dev_put(dev);
release_sock(sk); release_sock(sk);
rtnl_unlock(); rtnl_unlock();
@ -614,9 +606,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
rtnl_lock(); rtnl_lock();
lock_sock(sk); lock_sock(sk);
if (ro->bound && ro->ifindex) { dev = ro->dev;
dev = dev_get_by_index(sock_net(sk), ro->ifindex); if (ro->bound && dev) {
if (!dev) { if (dev->reg_state != NETREG_REGISTERED) {
err = -ENODEV; err = -ENODEV;
goto out_err; goto out_err;
} }
@ -640,7 +632,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
ro->err_mask = err_mask; ro->err_mask = err_mask;
out_err: out_err:
dev_put(dev);
release_sock(sk); release_sock(sk);
rtnl_unlock(); rtnl_unlock();