| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANn89i+FBa-KLJz5xPvk3jO3Miww4Vs+qw4nPf_9SPwiWpyTWw@mail.gmail.com>
Date: Wed, 27 Jul 2022 11:06:08 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Bernard Pidoux <f6bvp@...e.fr>
Cc: Jakub Kicinski <kuba@...nel.org>,
David Miller <davem@...emloft.net>,
Duoming Zhou <duoming@....edu.cn>, linux-hams@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>,
netdev <netdev@...r.kernel.org>, Paolo Abeni <pabeni@...hat.com>,
Ralf Baechle <ralf@...ux-mips.org>
Subject: Re: [PATCH 1/1] [PATCH] net: rose: fix unregistered netdevice:
waiting for rose0 to become free
On Tue, Jul 26, 2022 at 8:25 PM Bernard Pidoux <f6bvp@...e.fr> wrote:
>
> Here is the context.
>
> This patch adds dev_put(dev) in order to allow removal of rose module
> after use of AX25 and ROSE via rose0 device.
>
> Otherwise when trying to remove rose module via rmmod rose an infinite
> loop message was displayed on all consoles with xx being a random number.
>
> unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
>
> unregistered_netdevice: waiting for rose0 to become free. Usage count = xx
>
> ...
>
> With the patch it is ok to rmmod rose.
But removing a net device will leave a dangling pointer, leading to UAF.
We must keep a reference and remove it when the socket is dismantled.
Also rose_dev_first() is buggy, because it leaves the rcu section
without taking first a reference on the found device.
Here is a probably not complete patch, can you give it a try ?
(Also enable CONFIG_NET_DEV_REFCNT_TRACKER=y in your .config to ease debugging)
(I can send you privately the patch, just ask me, I include it inline
here for clarity only)
Thanks.
diff --git a/include/net/rose.h b/include/net/rose.h
index 0f0a4ce0fee7cc5e125507a8fc3cfb8cb826be73..64f808eed0e15a2482e8ce010d712eef1e0b9d85
100644
--- a/include/net/rose.h
+++ b/include/net/rose.h
@@ -131,7 +131,8 @@ struct rose_sock {
ax25_address source_digis[ROSE_MAX_DIGIS];
ax25_address dest_digis[ROSE_MAX_DIGIS];
struct rose_neigh *neighbour;
- struct net_device *device;
+ struct net_device *device;
+ netdevice_tracker dev_tracker;
unsigned int lci, rand;
unsigned char state, condition, qbitincl, defer;
unsigned char cause, diagnostic;
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index bf2d986a6bc392a9d830b1dfa7fbaa3bca969aa3..520a48999f1bf8a41d66e8a4f86606b66f2b9408
100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -192,6 +192,7 @@ static void rose_kill_by_device(struct net_device *dev)
rose_disconnect(s, ENETUNREACH, ROSE_OUT_OF_ORDER, 0);
if (rose->neighbour)
rose->neighbour->use--;
+ dev_put_track(rose->device, &rose->dev_tracker);
rose->device = NULL;
}
}
@@ -592,6 +593,8 @@ static struct sock *rose_make_new(struct sock *osk)
rose->idle = orose->idle;
rose->defer = orose->defer;
rose->device = orose->device;
+ if (rose->device)
+ dev_hold_track(rose->device, &rose->dev_tracker, GFP_ATOMIC);
rose->qbitincl = orose->qbitincl;
return sk;
@@ -695,7 +698,11 @@ static int rose_bind(struct socket *sock, struct
sockaddr *uaddr, int addr_len)
}
rose->source_addr = addr->srose_addr;
+ // TODO: should probably hold socket lock at this point ?
+ WARN_ON_ONCE(rose->device);
rose->device = dev;
+ netdev_tracker_alloc(rose->device, &rose->dev_tracker, GFP_KERNEL);
+
rose->source_ndigis = addr->srose_ndigis;
if (addr_len == sizeof(struct full_sockaddr_rose)) {
@@ -721,7 +728,6 @@ static int rose_connect(struct socket *sock,
struct sockaddr *uaddr, int addr_le
struct rose_sock *rose = rose_sk(sk);
struct sockaddr_rose *addr = (struct sockaddr_rose *)uaddr;
unsigned char cause, diagnostic;
- struct net_device *dev;
ax25_uid_assoc *user;
int n, err = 0;
@@ -778,9 +784,12 @@ static int rose_connect(struct socket *sock,
struct sockaddr *uaddr, int addr_le
}
if (sock_flag(sk, SOCK_ZAPPED)) { /* Must bind first -
autobinding in this may or may not work */
+ struct net_device *dev;
+
sock_reset_flag(sk, SOCK_ZAPPED);
- if ((dev = rose_dev_first()) == NULL) {
+ dev = rose_dev_first();
+ if (!dev) {
err = -ENETUNREACH;
goto out_release;
}
@@ -788,12 +797,15 @@ static int rose_connect(struct socket *sock,
struct sockaddr *uaddr, int addr_le
user = ax25_findbyuid(current_euid());
if (!user) {
err = -EINVAL;
+ dev_put(dev);
goto out_release;
}
memcpy(&rose->source_addr, dev->dev_addr, ROSE_ADDR_LEN);
rose->source_call = user->call;
rose->device = dev;
+ netdev_tracker_alloc(rose->device, &rose->dev_tracker,
+ GFP_KERNEL);
ax25_uid_put(user);
rose_insert_socket(sk); /* Finish the bind */
@@ -1017,6 +1029,7 @@ int rose_rx_call_request(struct sk_buff *skb,
struct net_device *dev, struct ros
make_rose->source_digis[n] = facilities.source_digis[n];
make_rose->neighbour = neigh;
make_rose->device = dev;
+ dev_hold_track(make_rose->device, &make_rose->dev_tracker, GFP_ATOMIC);
make_rose->facilities = facilities;
make_rose->neighbour->use++;
Powered by blists - more mailing lists