summaryrefslogtreecommitdiff
path: root/net
diff options
context:
space:
mode:
authorDuoming Zhou <duoming@zju.edu.cn>2022-04-15 19:14:18 +0300
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2022-04-20 10:34:22 +0300
commit452ae92b99062d2f6a34324eaf705a3b7eac9f8b (patch)
tree2e00265f42441997d4e4fff2652a4b759b9a5cee /net
parentb982492ec3a115e0a136856a1b2dbe32f2d21a0e (diff)
downloadlinux-452ae92b99062d2f6a34324eaf705a3b7eac9f8b.tar.xz
ax25: Fix refcount leaks caused by ax25_cb_del()
commit 9fd75b66b8f68498454d685dc4ba13192ae069b0 upstream. The previous commit d01ffb9eee4a ("ax25: add refcount in ax25_dev to avoid UAF bugs") and commit feef318c855a ("ax25: fix UAF bugs of net_device caused by rebinding operation") increase the refcounts of ax25_dev and net_device in ax25_bind() and decrease the matching refcounts in ax25_kill_by_device() in order to prevent UAF bugs, but there are reference count leaks. The root cause of refcount leaks is shown below: (Thread 1) | (Thread 2) ax25_bind() | ... | ax25_addr_ax25dev() | ax25_dev_hold() //(1) | ... | dev_hold_track() //(2) | ... | ax25_destroy_socket() | ax25_cb_del() | ... | hlist_del_init() //(3) | | (Thread 3) | ax25_kill_by_device() | ... | ax25_for_each(s, &ax25_list) { | if (s->ax25_dev == ax25_dev) //(4) | ... | Firstly, we use ax25_bind() to increase the refcount of ax25_dev in position (1) and increase the refcount of net_device in position (2). Then, we use ax25_cb_del() invoked by ax25_destroy_socket() to delete ax25_cb in hlist in position (3) before calling ax25_kill_by_device(). Finally, the decrements of refcounts in ax25_kill_by_device() will not be executed, because no s->ax25_dev equals to ax25_dev in position (4). This patch adds decrements of refcounts in ax25_release() and use lock_sock() to do synchronization. If refcounts decrease in ax25_release(), the decrements of refcounts in ax25_kill_by_device() will not be executed and vice versa. Fixes: d01ffb9eee4a ("ax25: add refcount in ax25_dev to avoid UAF bugs") Fixes: 87563a043cef ("ax25: fix reference count leaks of ax25_dev") Fixes: feef318c855a ("ax25: fix UAF bugs of net_device caused by rebinding operation") Reported-by: Thomas Osterried <thomas@osterried.de> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> Signed-off-by: David S. Miller <davem@davemloft.net> [OP: backport to 5.15: adjust dev_put_track()->dev_put()] Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'net')
-rw-r--r--net/ax25/af_ax25.c14
1 files changed, 11 insertions, 3 deletions
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 1235bbcc7953..13e8c9a0cf4f 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -98,8 +98,10 @@ again:
spin_unlock_bh(&ax25_list_lock);
lock_sock(sk);
s->ax25_dev = NULL;
- dev_put(ax25_dev->dev);
- ax25_dev_put(ax25_dev);
+ if (sk->sk_socket) {
+ dev_put(ax25_dev->dev);
+ ax25_dev_put(ax25_dev);
+ }
release_sock(sk);
ax25_disconnect(s, ENETUNREACH);
spin_lock_bh(&ax25_list_lock);
@@ -979,14 +981,20 @@ static int ax25_release(struct socket *sock)
{
struct sock *sk = sock->sk;
ax25_cb *ax25;
+ ax25_dev *ax25_dev;
if (sk == NULL)
return 0;
sock_hold(sk);
- sock_orphan(sk);
lock_sock(sk);
+ sock_orphan(sk);
ax25 = sk_to_ax25(sk);
+ ax25_dev = ax25->ax25_dev;
+ if (ax25_dev) {
+ dev_put(ax25_dev->dev);
+ ax25_dev_put(ax25_dev);
+ }
if (sk->sk_type == SOCK_SEQPACKET) {
switch (ax25->state) {