mirror of
https://github.com/Fishwaldo/Star64_linux.git
synced 2025-03-16 04:04:06 +00:00
can: raw: fix receiver memory leak
[ Upstream commitee8b94c851
] 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> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
1fdbd5ddf3
commit
9411a38299
1 changed files with 24 additions and 33 deletions
|
@ -83,6 +83,7 @@ struct raw_sock {
|
|||
struct sock sk;
|
||||
int bound;
|
||||
int ifindex;
|
||||
struct net_device *dev;
|
||||
struct list_head notifier;
|
||||
int loopback;
|
||||
int recv_own_msgs;
|
||||
|
@ -275,7 +276,7 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg,
|
|||
if (!net_eq(dev_net(dev), sock_net(sk)))
|
||||
return;
|
||||
|
||||
if (ro->ifindex != dev->ifindex)
|
||||
if (ro->dev != dev)
|
||||
return;
|
||||
|
||||
switch (msg) {
|
||||
|
@ -290,6 +291,7 @@ static void raw_notify(struct raw_sock *ro, unsigned long msg,
|
|||
|
||||
ro->ifindex = 0;
|
||||
ro->bound = 0;
|
||||
ro->dev = NULL;
|
||||
ro->count = 0;
|
||||
release_sock(sk);
|
||||
|
||||
|
@ -335,6 +337,7 @@ static int raw_init(struct sock *sk)
|
|||
|
||||
ro->bound = 0;
|
||||
ro->ifindex = 0;
|
||||
ro->dev = NULL;
|
||||
|
||||
/* set default filter to single entry dfilter */
|
||||
ro->dfilter.can_id = 0;
|
||||
|
@ -382,19 +385,13 @@ static int raw_release(struct socket *sock)
|
|||
|
||||
lock_sock(sk);
|
||||
|
||||
rtnl_lock();
|
||||
/* remove current filters & unregister */
|
||||
if (ro->bound) {
|
||||
if (ro->ifindex) {
|
||||
struct net_device *dev;
|
||||
|
||||
dev = dev_get_by_index(sock_net(sk), ro->ifindex);
|
||||
if (dev) {
|
||||
raw_disable_allfilters(dev_net(dev), dev, sk);
|
||||
dev_put(dev);
|
||||
}
|
||||
} else {
|
||||
if (ro->dev)
|
||||
raw_disable_allfilters(dev_net(ro->dev), ro->dev, sk);
|
||||
else
|
||||
raw_disable_allfilters(sock_net(sk), NULL, sk);
|
||||
}
|
||||
}
|
||||
|
||||
if (ro->count > 1)
|
||||
|
@ -402,8 +399,10 @@ static int raw_release(struct socket *sock)
|
|||
|
||||
ro->ifindex = 0;
|
||||
ro->bound = 0;
|
||||
ro->dev = NULL;
|
||||
ro->count = 0;
|
||||
free_percpu(ro->uniq);
|
||||
rtnl_unlock();
|
||||
|
||||
sock_orphan(sk);
|
||||
sock->sk = NULL;
|
||||
|
@ -419,6 +418,7 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len)
|
|||
struct sockaddr_can *addr = (struct sockaddr_can *)uaddr;
|
||||
struct sock *sk = sock->sk;
|
||||
struct raw_sock *ro = raw_sk(sk);
|
||||
struct net_device *dev = NULL;
|
||||
int ifindex;
|
||||
int err = 0;
|
||||
int notify_enetdown = 0;
|
||||
|
@ -428,14 +428,13 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len)
|
|||
if (addr->can_family != AF_CAN)
|
||||
return -EINVAL;
|
||||
|
||||
rtnl_lock();
|
||||
lock_sock(sk);
|
||||
|
||||
if (ro->bound && addr->can_ifindex == ro->ifindex)
|
||||
goto out;
|
||||
|
||||
if (addr->can_ifindex) {
|
||||
struct net_device *dev;
|
||||
|
||||
dev = dev_get_by_index(sock_net(sk), addr->can_ifindex);
|
||||
if (!dev) {
|
||||
err = -ENODEV;
|
||||
|
@ -464,26 +463,20 @@ static int raw_bind(struct socket *sock, struct sockaddr *uaddr, int len)
|
|||
if (!err) {
|
||||
if (ro->bound) {
|
||||
/* unregister old filters */
|
||||
if (ro->ifindex) {
|
||||
struct net_device *dev;
|
||||
|
||||
dev = dev_get_by_index(sock_net(sk),
|
||||
ro->ifindex);
|
||||
if (dev) {
|
||||
raw_disable_allfilters(dev_net(dev),
|
||||
dev, sk);
|
||||
dev_put(dev);
|
||||
}
|
||||
} else {
|
||||
if (ro->dev)
|
||||
raw_disable_allfilters(dev_net(ro->dev),
|
||||
ro->dev, sk);
|
||||
else
|
||||
raw_disable_allfilters(sock_net(sk), NULL, sk);
|
||||
}
|
||||
}
|
||||
ro->ifindex = ifindex;
|
||||
ro->bound = 1;
|
||||
ro->dev = dev;
|
||||
}
|
||||
|
||||
out:
|
||||
release_sock(sk);
|
||||
rtnl_unlock();
|
||||
|
||||
if (notify_enetdown) {
|
||||
sk->sk_err = ENETDOWN;
|
||||
|
@ -549,9 +542,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
|
|||
rtnl_lock();
|
||||
lock_sock(sk);
|
||||
|
||||
if (ro->bound && ro->ifindex) {
|
||||
dev = dev_get_by_index(sock_net(sk), ro->ifindex);
|
||||
if (!dev) {
|
||||
dev = ro->dev;
|
||||
if (ro->bound && dev) {
|
||||
if (dev->reg_state != NETREG_REGISTERED) {
|
||||
if (count > 1)
|
||||
kfree(filter);
|
||||
err = -ENODEV;
|
||||
|
@ -592,7 +585,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
|
|||
ro->count = count;
|
||||
|
||||
out_fil:
|
||||
dev_put(dev);
|
||||
release_sock(sk);
|
||||
rtnl_unlock();
|
||||
|
||||
|
@ -610,9 +602,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
|
|||
rtnl_lock();
|
||||
lock_sock(sk);
|
||||
|
||||
if (ro->bound && ro->ifindex) {
|
||||
dev = dev_get_by_index(sock_net(sk), ro->ifindex);
|
||||
if (!dev) {
|
||||
dev = ro->dev;
|
||||
if (ro->bound && dev) {
|
||||
if (dev->reg_state != NETREG_REGISTERED) {
|
||||
err = -ENODEV;
|
||||
goto out_err;
|
||||
}
|
||||
|
@ -636,7 +628,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
|
|||
ro->err_mask = err_mask;
|
||||
|
||||
out_err:
|
||||
dev_put(dev);
|
||||
release_sock(sk);
|
||||
rtnl_unlock();
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue