* [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
@ 2023-06-10 1:34 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2023-06-10 1:34 UTC (permalink / raw)
To: arnd, bridge, davem, edumazet, kuba, linux-kernel, netdev,
nikolay, pabeni, roopa, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 67faabbde36b selftests/bpf: Add missing prototypes for sev..
git tree: bpf-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1381363b280000
kernel config: https://syzkaller.appspot.com/x/.config?x=5335204dcdecfda
dashboard link: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=132faf93280000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10532add280000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/751a0490d875/disk-67faabbd.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/2c5106cd9f1f/vmlinux-67faabbd.xz
kernel image: https://storage.googleapis.com/syzbot-assets/62c1154294e4/bzImage-67faabbd.xz
The issue was bisected to:
commit ad2f99aedf8fa77f3ae647153284fa63c43d3055
Author: Arnd Bergmann <arnd@arndb.de>
Date: Tue Jul 27 13:45:16 2021 +0000
net: bridge: move bridge ioctls out of .ndo_do_ioctl
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=146de6f1280000
final oops: https://syzkaller.appspot.com/x/report.txt?x=166de6f1280000
console output: https://syzkaller.appspot.com/x/log.txt?x=126de6f1280000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
Fixes: ad2f99aedf8f ("net: bridge: move bridge ioctls out of .ndo_do_ioctl")
unregister_netdevice: waiting for bridge0 to become free. Usage count = 2
leaked reference.
__netdev_tracker_alloc include/linux/netdevice.h:4070 [inline]
netdev_hold include/linux/netdevice.h:4099 [inline]
dev_ifsioc+0xbc0/0xeb0 net/core/dev_ioctl.c:408
dev_ioctl+0x250/0x1090 net/core/dev_ioctl.c:605
sock_do_ioctl+0x15a/0x230 net/socket.c:1215
sock_ioctl+0x1f8/0x680 net/socket.c:1318
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:870 [inline]
__se_sys_ioctl fs/ioctl.c:856 [inline]
__x64_sys_ioctl+0x197/0x210 fs/ioctl.c:856
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Bridge] [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
2023-06-10 1:34 ` syzbot
@ 2023-06-21 7:07 ` Ziqi Zhao
-1 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-06-21 7:07 UTC (permalink / raw)
To: syzbot+881d65229ca4f9ae8c84
Cc: ivan.orlov0322, arnd, netdev, bridge, syzkaller-bugs,
linux-kernel, edumazet, nikolay, roopa, kuba, skhan, pabeni,
davem
Hi all,
I'm taking a look at this bug as part of the exercice for the Linux
Kernel Bug Fixing Summer 2023 program. Thanks to the help from my
mentor, Ivan Orlov and Shuah Khan, I've already obtained a reproduction
of the issue using the provided C reproducer, and I should be able to
submit a patch by the end of this week to fix the highlighted error. If
you have any information or suggestions, please feel free to reply to
this thread. Any help would be greatly appreciated!
Best regards,
Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
@ 2023-06-21 7:07 ` Ziqi Zhao
0 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-06-21 7:07 UTC (permalink / raw)
To: syzbot+881d65229ca4f9ae8c84
Cc: arnd, bridge, davem, edumazet, kuba, linux-kernel, netdev,
nikolay, pabeni, roopa, syzkaller-bugs, skhan, ivan.orlov0322
Hi all,
I'm taking a look at this bug as part of the exercice for the Linux
Kernel Bug Fixing Summer 2023 program. Thanks to the help from my
mentor, Ivan Orlov and Shuah Khan, I've already obtained a reproduction
of the issue using the provided C reproducer, and I should be able to
submit a patch by the end of this week to fix the highlighted error. If
you have any information or suggestions, please feel free to reply to
this thread. Any help would be greatly appreciated!
Best regards,
Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Bridge] [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
2023-06-21 7:07 ` Ziqi Zhao
@ 2023-06-21 8:46 ` Dongliang Mu
-1 siblings, 0 replies; 23+ messages in thread
From: Dongliang Mu @ 2023-06-21 8:46 UTC (permalink / raw)
To: Ziqi Zhao
Cc: ivan.orlov0322, arnd, netdev, bridge, syzkaller-bugs,
linux-kernel, edumazet, nikolay, roopa,
syzbot+881d65229ca4f9ae8c84, kuba, skhan, pabeni, davem
On Wed, Jun 21, 2023 at 3:38 PM 'Ziqi Zhao' via syzkaller-bugs
<syzkaller-bugs@googlegroups.com> wrote:
>
> Hi all,
>
> I'm taking a look at this bug as part of the exercice for the Linux
> Kernel Bug Fixing Summer 2023 program. Thanks to the help from my
This is an interesting program. There are many kernel crashes on the
syzbot dashboard, which needs help.
> mentor, Ivan Orlov and Shuah Khan, I've already obtained a reproduction
> of the issue using the provided C reproducer, and I should be able to
> submit a patch by the end of this week to fix the highlighted error. If
> you have any information or suggestions, please feel free to reply to
> this thread. Any help would be greatly appreciated!
Please carefully read the guidance of submitting patches to linux
kernel [1]. Be careful about your coding style before sending.
Note that, Syzbot has the feature: patch testing. You can upload and
test your own patch to confirm that your patch is working properly.
[1] https://docs.kernel.org/process/submitting-patches.html
>
> Best regards,
> Ziqi
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20230621070710.380373-1-astrajoan%40yahoo.com.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
@ 2023-06-21 8:46 ` Dongliang Mu
0 siblings, 0 replies; 23+ messages in thread
From: Dongliang Mu @ 2023-06-21 8:46 UTC (permalink / raw)
To: Ziqi Zhao
Cc: syzbot+881d65229ca4f9ae8c84, arnd, bridge, davem, edumazet, kuba,
linux-kernel, netdev, nikolay, pabeni, roopa, syzkaller-bugs,
skhan, ivan.orlov0322
On Wed, Jun 21, 2023 at 3:38 PM 'Ziqi Zhao' via syzkaller-bugs
<syzkaller-bugs@googlegroups.com> wrote:
>
> Hi all,
>
> I'm taking a look at this bug as part of the exercice for the Linux
> Kernel Bug Fixing Summer 2023 program. Thanks to the help from my
This is an interesting program. There are many kernel crashes on the
syzbot dashboard, which needs help.
> mentor, Ivan Orlov and Shuah Khan, I've already obtained a reproduction
> of the issue using the provided C reproducer, and I should be able to
> submit a patch by the end of this week to fix the highlighted error. If
> you have any information or suggestions, please feel free to reply to
> this thread. Any help would be greatly appreciated!
Please carefully read the guidance of submitting patches to linux
kernel [1]. Be careful about your coding style before sending.
Note that, Syzbot has the feature: patch testing. You can upload and
test your own patch to confirm that your patch is working properly.
[1] https://docs.kernel.org/process/submitting-patches.html
>
> Best regards,
> Ziqi
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20230621070710.380373-1-astrajoan%40yahoo.com.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Bridge] [PATCH] can: j1939: prevent deadlock by changing j1939_socks_lock to rwlock
2023-06-21 8:46 ` Dongliang Mu
@ 2023-06-26 5:50 ` Ziqi Zhao
-1 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-06-26 5:50 UTC (permalink / raw)
To: mudongliangabcd
Cc: ivan.orlov0322, arnd, roopa, astrajoan, bridge, syzkaller-bugs,
linux-kernel, edumazet, nikolay, netdev,
syzbot+881d65229ca4f9ae8c84, kuba, skhan, pabeni, davem
The following 3 locks would race against each other, causing the
deadlock situation in the Syzbot bug report:
- j1939_socks_lock
- active_session_list_lock
- sk_session_queue_lock
A reasonable fix is to change j1939_socks_lock to an rwlock, since in
the rare situations where a write lock is required for the linked list
that j1939_socks_lock is protecting, the code does not attempt to
acquire any more locks. This would break the circular lock dependency,
where, for example, the current thread already locks j1939_socks_lock
and attempts to acquire sk_session_queue_lock, and at the same time,
another thread attempts to acquire j1939_socks_lock while holding
sk_session_queue_lock.
NOTE: This patch along does not fix the unregister_netdevice bug
reported by Syzbot; instead, it solves a deadlock situation to prepare
for one or more further patches to actually fix the Syzbot bug, which
appears to be a reference counting problem within the j1939 codebase.
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
net/can/j1939/j1939-priv.h | 2 +-
net/can/j1939/main.c | 2 +-
net/can/j1939/socket.c | 25 +++++++++++++------------
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
index 16af1a7f80f6..74f15592d170 100644
--- a/net/can/j1939/j1939-priv.h
+++ b/net/can/j1939/j1939-priv.h
@@ -86,7 +86,7 @@ struct j1939_priv {
unsigned int tp_max_packet_size;
/* lock for j1939_socks list */
- spinlock_t j1939_socks_lock;
+ rwlock_t j1939_socks_lock;
struct list_head j1939_socks;
struct kref rx_kref;
diff --git a/net/can/j1939/main.c b/net/can/j1939/main.c
index ecff1c947d68..a6fb89fa6278 100644
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -274,7 +274,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
return ERR_PTR(-ENOMEM);
j1939_tp_init(priv);
- spin_lock_init(&priv->j1939_socks_lock);
+ rwlock_init(&priv->j1939_socks_lock);
INIT_LIST_HEAD(&priv->j1939_socks);
mutex_lock(&j1939_netdev_lock);
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 35970c25496a..6dce9d645116 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -80,16 +80,16 @@ static void j1939_jsk_add(struct j1939_priv *priv, struct j1939_sock *jsk)
jsk->state |= J1939_SOCK_BOUND;
j1939_priv_get(priv);
- spin_lock_bh(&priv->j1939_socks_lock);
+ write_lock_bh(&priv->j1939_socks_lock);
list_add_tail(&jsk->list, &priv->j1939_socks);
- spin_unlock_bh(&priv->j1939_socks_lock);
+ write_unlock_bh(&priv->j1939_socks_lock);
}
static void j1939_jsk_del(struct j1939_priv *priv, struct j1939_sock *jsk)
{
- spin_lock_bh(&priv->j1939_socks_lock);
+ write_lock_bh(&priv->j1939_socks_lock);
list_del_init(&jsk->list);
- spin_unlock_bh(&priv->j1939_socks_lock);
+ write_unlock_bh(&priv->j1939_socks_lock);
j1939_priv_put(priv);
jsk->state &= ~J1939_SOCK_BOUND;
@@ -329,13 +329,13 @@ bool j1939_sk_recv_match(struct j1939_priv *priv, struct j1939_sk_buff_cb *skcb)
struct j1939_sock *jsk;
bool match = false;
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
match = j1939_sk_recv_match_one(jsk, skcb);
if (match)
break;
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
return match;
}
@@ -344,11 +344,11 @@ void j1939_sk_recv(struct j1939_priv *priv, struct sk_buff *skb)
{
struct j1939_sock *jsk;
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
j1939_sk_recv_one(jsk, skb);
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
}
static void j1939_sk_sock_destruct(struct sock *sk)
@@ -484,6 +484,7 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
priv = j1939_netdev_start(ndev);
dev_put(ndev);
+
if (IS_ERR(priv)) {
ret = PTR_ERR(priv);
goto out_release_sock;
@@ -1078,12 +1079,12 @@ void j1939_sk_errqueue(struct j1939_session *session,
}
/* spread RX notifications to all sockets subscribed to this session */
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
if (j1939_sk_recv_match_one(jsk, &session->skcb))
__j1939_sk_errqueue(session, &jsk->sk, type);
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
};
void j1939_sk_send_loop_abort(struct sock *sk, int err)
@@ -1271,7 +1272,7 @@ void j1939_sk_netdev_event_netdown(struct j1939_priv *priv)
struct j1939_sock *jsk;
int error_code = ENETDOWN;
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
jsk->sk.sk_err = error_code;
if (!sock_flag(&jsk->sk, SOCK_DEAD))
@@ -1279,7 +1280,7 @@ void j1939_sk_netdev_event_netdown(struct j1939_priv *priv)
j1939_sk_queue_drop_all(priv, jsk, error_code);
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
}
static int j1939_sk_no_ioctlcmd(struct socket *sock, unsigned int cmd,
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH] can: j1939: prevent deadlock by changing j1939_socks_lock to rwlock
@ 2023-06-26 5:50 ` Ziqi Zhao
0 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-06-26 5:50 UTC (permalink / raw)
To: mudongliangabcd
Cc: arnd, astrajoan, bridge, davem, edumazet, ivan.orlov0322, kuba,
linux-kernel, netdev, nikolay, pabeni, roopa, skhan,
syzbot+881d65229ca4f9ae8c84, syzkaller-bugs
The following 3 locks would race against each other, causing the
deadlock situation in the Syzbot bug report:
- j1939_socks_lock
- active_session_list_lock
- sk_session_queue_lock
A reasonable fix is to change j1939_socks_lock to an rwlock, since in
the rare situations where a write lock is required for the linked list
that j1939_socks_lock is protecting, the code does not attempt to
acquire any more locks. This would break the circular lock dependency,
where, for example, the current thread already locks j1939_socks_lock
and attempts to acquire sk_session_queue_lock, and at the same time,
another thread attempts to acquire j1939_socks_lock while holding
sk_session_queue_lock.
NOTE: This patch along does not fix the unregister_netdevice bug
reported by Syzbot; instead, it solves a deadlock situation to prepare
for one or more further patches to actually fix the Syzbot bug, which
appears to be a reference counting problem within the j1939 codebase.
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
net/can/j1939/j1939-priv.h | 2 +-
net/can/j1939/main.c | 2 +-
net/can/j1939/socket.c | 25 +++++++++++++------------
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
index 16af1a7f80f6..74f15592d170 100644
--- a/net/can/j1939/j1939-priv.h
+++ b/net/can/j1939/j1939-priv.h
@@ -86,7 +86,7 @@ struct j1939_priv {
unsigned int tp_max_packet_size;
/* lock for j1939_socks list */
- spinlock_t j1939_socks_lock;
+ rwlock_t j1939_socks_lock;
struct list_head j1939_socks;
struct kref rx_kref;
diff --git a/net/can/j1939/main.c b/net/can/j1939/main.c
index ecff1c947d68..a6fb89fa6278 100644
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -274,7 +274,7 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
return ERR_PTR(-ENOMEM);
j1939_tp_init(priv);
- spin_lock_init(&priv->j1939_socks_lock);
+ rwlock_init(&priv->j1939_socks_lock);
INIT_LIST_HEAD(&priv->j1939_socks);
mutex_lock(&j1939_netdev_lock);
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index 35970c25496a..6dce9d645116 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -80,16 +80,16 @@ static void j1939_jsk_add(struct j1939_priv *priv, struct j1939_sock *jsk)
jsk->state |= J1939_SOCK_BOUND;
j1939_priv_get(priv);
- spin_lock_bh(&priv->j1939_socks_lock);
+ write_lock_bh(&priv->j1939_socks_lock);
list_add_tail(&jsk->list, &priv->j1939_socks);
- spin_unlock_bh(&priv->j1939_socks_lock);
+ write_unlock_bh(&priv->j1939_socks_lock);
}
static void j1939_jsk_del(struct j1939_priv *priv, struct j1939_sock *jsk)
{
- spin_lock_bh(&priv->j1939_socks_lock);
+ write_lock_bh(&priv->j1939_socks_lock);
list_del_init(&jsk->list);
- spin_unlock_bh(&priv->j1939_socks_lock);
+ write_unlock_bh(&priv->j1939_socks_lock);
j1939_priv_put(priv);
jsk->state &= ~J1939_SOCK_BOUND;
@@ -329,13 +329,13 @@ bool j1939_sk_recv_match(struct j1939_priv *priv, struct j1939_sk_buff_cb *skcb)
struct j1939_sock *jsk;
bool match = false;
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
match = j1939_sk_recv_match_one(jsk, skcb);
if (match)
break;
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
return match;
}
@@ -344,11 +344,11 @@ void j1939_sk_recv(struct j1939_priv *priv, struct sk_buff *skb)
{
struct j1939_sock *jsk;
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
j1939_sk_recv_one(jsk, skb);
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
}
static void j1939_sk_sock_destruct(struct sock *sk)
@@ -484,6 +484,7 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
priv = j1939_netdev_start(ndev);
dev_put(ndev);
+
if (IS_ERR(priv)) {
ret = PTR_ERR(priv);
goto out_release_sock;
@@ -1078,12 +1079,12 @@ void j1939_sk_errqueue(struct j1939_session *session,
}
/* spread RX notifications to all sockets subscribed to this session */
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
if (j1939_sk_recv_match_one(jsk, &session->skcb))
__j1939_sk_errqueue(session, &jsk->sk, type);
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
};
void j1939_sk_send_loop_abort(struct sock *sk, int err)
@@ -1271,7 +1272,7 @@ void j1939_sk_netdev_event_netdown(struct j1939_priv *priv)
struct j1939_sock *jsk;
int error_code = ENETDOWN;
- spin_lock_bh(&priv->j1939_socks_lock);
+ read_lock_bh(&priv->j1939_socks_lock);
list_for_each_entry(jsk, &priv->j1939_socks, list) {
jsk->sk.sk_err = error_code;
if (!sock_flag(&jsk->sk, SOCK_DEAD))
@@ -1279,7 +1280,7 @@ void j1939_sk_netdev_event_netdown(struct j1939_priv *priv)
j1939_sk_queue_drop_all(priv, jsk, error_code);
}
- spin_unlock_bh(&priv->j1939_socks_lock);
+ read_unlock_bh(&priv->j1939_socks_lock);
}
static int j1939_sk_no_ioctlcmd(struct socket *sock, unsigned int cmd,
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Bridge] [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
2023-06-10 1:34 ` syzbot
@ 2023-08-19 8:10 ` Ziqi Zhao
-1 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-08-19 8:10 UTC (permalink / raw)
To: arnd, bridge, davem, edumazet, f.fainelli, ivan.orlov0322,
keescook, kuba, hkallweit1, mudongliangabcd, nikolay, pabeni,
razor, roopa, skhan, syzbot+881d65229ca4f9ae8c84, vladimir.oltean
Cc: netdev, syzkaller-bugs, linux-kernel, Ziqi Zhao
In the bug reported by Syzbot, certain bridge devices would have a
leaked reference created by race conditions in dev_ioctl, specifically,
under SIOCBRADDIF or SIOCBRDELIF operations. The reference leak would
be shown in the periodic unregister_netdevice call, which throws a
warning and cause Syzbot to report a crash. Upon inspection of the
logic in dev_ioctl, it seems the reference was introduced to ensure
proper access to the bridge device after rtnl_unlock. and the latter
function is necessary to maintain the following lock order in any
bridge related ioctl calls:
1) br_ioctl_mutex => 2) rtnl_lock
Conceptually, though, br_ioctl_mutex could be considered more specific
than rtnl_lock given their usages, hence swapping their order would be
a reasonable proposal. This patch changes all related call sites to
maintain the reversed order of the two locks:
1) rtnl_lock => 2) br_ioctl_mutex
By doing so, the extra reference introduced in dev_ioctl is no longer
needed, and hence the reference leak bug is now resolved.
Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
Fixes: ad2f99aedf8f ("net: bridge: move bridge ioctls out of .ndo_do_ioctl")
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
net/bridge/br_ioctl.c | 4 ----
net/core/dev_ioctl.c | 8 +-------
net/socket.c | 2 ++
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index f213ed108361..291dbc5d2a99 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -399,8 +399,6 @@ int br_ioctl_stub(struct net *net, struct net_bridge *br, unsigned int cmd,
{
int ret = -EOPNOTSUPP;
- rtnl_lock();
-
switch (cmd) {
case SIOCGIFBR:
case SIOCSIFBR:
@@ -434,7 +432,5 @@ int br_ioctl_stub(struct net *net, struct net_bridge *br, unsigned int cmd,
break;
}
- rtnl_unlock();
-
return ret;
}
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 3730945ee294..17df956df8cb 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -336,7 +336,6 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
int err;
struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
const struct net_device_ops *ops;
- netdevice_tracker dev_tracker;
if (!dev)
return -ENODEV;
@@ -405,12 +404,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
return -ENODEV;
if (!netif_is_bridge_master(dev))
return -EOPNOTSUPP;
- netdev_hold(dev, &dev_tracker, GFP_KERNEL);
- rtnl_unlock();
- err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL);
- netdev_put(dev, &dev_tracker);
- rtnl_lock();
- return err;
+ return br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL);
case SIOCDEVPRIVATE ... SIOCDEVPRIVATE + 15:
return dev_siocdevprivate(dev, ifr, data, cmd);
diff --git a/net/socket.c b/net/socket.c
index 2b0e54b2405c..6b7a9df9a326 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1258,7 +1258,9 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
case SIOCSIFBR:
case SIOCBRADDBR:
case SIOCBRDELBR:
+ rtnl_lock();
err = br_ioctl_call(net, NULL, cmd, NULL, argp);
+ rtnl_unlock();
break;
case SIOCGIFVLAN:
case SIOCSIFVLAN:
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
@ 2023-08-19 8:10 ` Ziqi Zhao
0 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-08-19 8:10 UTC (permalink / raw)
To: arnd, bridge, davem, edumazet, f.fainelli, ivan.orlov0322,
keescook, kuba, hkallweit1, mudongliangabcd, nikolay, pabeni,
razor, roopa, skhan, syzbot+881d65229ca4f9ae8c84, vladimir.oltean
Cc: linux-kernel, netdev, syzkaller-bugs, Ziqi Zhao
In the bug reported by Syzbot, certain bridge devices would have a
leaked reference created by race conditions in dev_ioctl, specifically,
under SIOCBRADDIF or SIOCBRDELIF operations. The reference leak would
be shown in the periodic unregister_netdevice call, which throws a
warning and cause Syzbot to report a crash. Upon inspection of the
logic in dev_ioctl, it seems the reference was introduced to ensure
proper access to the bridge device after rtnl_unlock. and the latter
function is necessary to maintain the following lock order in any
bridge related ioctl calls:
1) br_ioctl_mutex => 2) rtnl_lock
Conceptually, though, br_ioctl_mutex could be considered more specific
than rtnl_lock given their usages, hence swapping their order would be
a reasonable proposal. This patch changes all related call sites to
maintain the reversed order of the two locks:
1) rtnl_lock => 2) br_ioctl_mutex
By doing so, the extra reference introduced in dev_ioctl is no longer
needed, and hence the reference leak bug is now resolved.
Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
Fixes: ad2f99aedf8f ("net: bridge: move bridge ioctls out of .ndo_do_ioctl")
Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
---
net/bridge/br_ioctl.c | 4 ----
net/core/dev_ioctl.c | 8 +-------
net/socket.c | 2 ++
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index f213ed108361..291dbc5d2a99 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -399,8 +399,6 @@ int br_ioctl_stub(struct net *net, struct net_bridge *br, unsigned int cmd,
{
int ret = -EOPNOTSUPP;
- rtnl_lock();
-
switch (cmd) {
case SIOCGIFBR:
case SIOCSIFBR:
@@ -434,7 +432,5 @@ int br_ioctl_stub(struct net *net, struct net_bridge *br, unsigned int cmd,
break;
}
- rtnl_unlock();
-
return ret;
}
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 3730945ee294..17df956df8cb 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -336,7 +336,6 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
int err;
struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
const struct net_device_ops *ops;
- netdevice_tracker dev_tracker;
if (!dev)
return -ENODEV;
@@ -405,12 +404,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
return -ENODEV;
if (!netif_is_bridge_master(dev))
return -EOPNOTSUPP;
- netdev_hold(dev, &dev_tracker, GFP_KERNEL);
- rtnl_unlock();
- err = br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL);
- netdev_put(dev, &dev_tracker);
- rtnl_lock();
- return err;
+ return br_ioctl_call(net, netdev_priv(dev), cmd, ifr, NULL);
case SIOCDEVPRIVATE ... SIOCDEVPRIVATE + 15:
return dev_siocdevprivate(dev, ifr, data, cmd);
diff --git a/net/socket.c b/net/socket.c
index 2b0e54b2405c..6b7a9df9a326 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1258,7 +1258,9 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
case SIOCSIFBR:
case SIOCBRADDBR:
case SIOCBRDELBR:
+ rtnl_lock();
err = br_ioctl_call(net, NULL, cmd, NULL, argp);
+ rtnl_unlock();
break;
case SIOCGIFVLAN:
case SIOCSIFVLAN:
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Bridge] [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
2023-08-19 8:10 ` Ziqi Zhao
@ 2023-08-19 9:25 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 23+ messages in thread
From: Nikolay Aleksandrov @ 2023-08-19 9:25 UTC (permalink / raw)
To: Ziqi Zhao, arnd, bridge, davem, edumazet, f.fainelli,
ivan.orlov0322, keescook, kuba, hkallweit1, mudongliangabcd,
nikolay, pabeni, roopa, skhan, syzbot+881d65229ca4f9ae8c84,
vladimir.oltean
Cc: netdev, syzkaller-bugs, linux-kernel
Hi Ziqi,
On 8/19/23 11:10, Ziqi Zhao wrote:
> In the bug reported by Syzbot, certain bridge devices would have a
> leaked reference created by race conditions in dev_ioctl, specifically,
> under SIOCBRADDIF or SIOCBRDELIF operations. The reference leak would
How would it leak a reference, could you elaborate?
The reference is always taken and always released after the call.
> be shown in the periodic unregister_netdevice call, which throws a
> warning and cause Syzbot to report a crash. Upon inspection of the
If you reproduced it, is the device later removed or is it really stuck?
> logic in dev_ioctl, it seems the reference was introduced to ensure
> proper access to the bridge device after rtnl_unlock. and the latter
> function is necessary to maintain the following lock order in any
> bridge related ioctl calls:
>
> 1) br_ioctl_mutex => 2) rtnl_lock
>
> Conceptually, though, br_ioctl_mutex could be considered more specific
> than rtnl_lock given their usages, hence swapping their order would be
> a reasonable proposal. This patch changes all related call sites to
> maintain the reversed order of the two locks:
>
> 1) rtnl_lock => 2) br_ioctl_mutex
>
> By doing so, the extra reference introduced in dev_ioctl is no longer
> needed, and hence the reference leak bug is now resolved.
IIRC there was no bug, it was a false-positive. The reference is held a
bit longer but then released, so the device is deleted later.
I might be remembering wrong, but I think I briefly looked into this
when it was reported. If that's not the case I'd be interested to see
a new report/trace because the bug might be somewhere else.
>
> Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
> Fixes: ad2f99aedf8f ("net: bridge: move bridge ioctls out of .ndo_do_ioctl")
> Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
> ---
> net/bridge/br_ioctl.c | 4 ----
> net/core/dev_ioctl.c | 8 +-------
> net/socket.c | 2 ++
> 3 files changed, 3 insertions(+), 11 deletions(-)
>
Thanks,
Nik
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
@ 2023-08-19 9:25 ` Nikolay Aleksandrov
0 siblings, 0 replies; 23+ messages in thread
From: Nikolay Aleksandrov @ 2023-08-19 9:25 UTC (permalink / raw)
To: Ziqi Zhao, arnd, bridge, davem, edumazet, f.fainelli,
ivan.orlov0322, keescook, kuba, hkallweit1, mudongliangabcd,
nikolay, pabeni, roopa, skhan, syzbot+881d65229ca4f9ae8c84,
vladimir.oltean
Cc: linux-kernel, netdev, syzkaller-bugs
Hi Ziqi,
On 8/19/23 11:10, Ziqi Zhao wrote:
> In the bug reported by Syzbot, certain bridge devices would have a
> leaked reference created by race conditions in dev_ioctl, specifically,
> under SIOCBRADDIF or SIOCBRDELIF operations. The reference leak would
How would it leak a reference, could you elaborate?
The reference is always taken and always released after the call.
> be shown in the periodic unregister_netdevice call, which throws a
> warning and cause Syzbot to report a crash. Upon inspection of the
If you reproduced it, is the device later removed or is it really stuck?
> logic in dev_ioctl, it seems the reference was introduced to ensure
> proper access to the bridge device after rtnl_unlock. and the latter
> function is necessary to maintain the following lock order in any
> bridge related ioctl calls:
>
> 1) br_ioctl_mutex => 2) rtnl_lock
>
> Conceptually, though, br_ioctl_mutex could be considered more specific
> than rtnl_lock given their usages, hence swapping their order would be
> a reasonable proposal. This patch changes all related call sites to
> maintain the reversed order of the two locks:
>
> 1) rtnl_lock => 2) br_ioctl_mutex
>
> By doing so, the extra reference introduced in dev_ioctl is no longer
> needed, and hence the reference leak bug is now resolved.
IIRC there was no bug, it was a false-positive. The reference is held a
bit longer but then released, so the device is deleted later.
I might be remembering wrong, but I think I briefly looked into this
when it was reported. If that's not the case I'd be interested to see
a new report/trace because the bug might be somewhere else.
>
> Reported-by: syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com
> Fixes: ad2f99aedf8f ("net: bridge: move bridge ioctls out of .ndo_do_ioctl")
> Signed-off-by: Ziqi Zhao <astrajoan@yahoo.com>
> ---
> net/bridge/br_ioctl.c | 4 ----
> net/core/dev_ioctl.c | 8 +-------
> net/socket.c | 2 ++
> 3 files changed, 3 insertions(+), 11 deletions(-)
>
Thanks,
Nik
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [Bridge] [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
2023-08-19 9:25 ` Nikolay Aleksandrov
@ 2023-08-19 22:50 ` Ziqi Zhao
-1 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-08-19 22:50 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: f.fainelli, ivan.orlov0322, keescook, arnd, vladimir.oltean,
bridge, syzkaller-bugs, mudongliangabcd, linux-kernel, edumazet,
netdev, nikolay, roopa, syzbot+881d65229ca4f9ae8c84, kuba, skhan,
pabeni, davem, hkallweit1
On Sat, Aug 19, 2023 at 12:25:15PM +0300, Nikolay Aleksandrov wrote:
Hi Nik,
Thank you so much for reviewing the patch and getting back to me!
> IIRC there was no bug, it was a false-positive. The reference is held a bit
> longer but then released, so the device is deleted later.
> If you reproduced it, is the device later removed or is it really stuck?
I ran the reproducer again without the patch and it seems you are
correct. It was trying to create a very short-lived bridge, then delete
it immediately in the next call. The device in question "wpan4" never
showed up when I polled with `ip link` in the VM, so I'd say it did not
get stuck.
> How would it leak a reference, could you elaborate?
> The reference is always taken and always released after the call.
This was where I got a bit confused too. The system had a timeout of
140 seconds for the unregister_netdevice check. If the bridge in
question was created and deleted repeatedly, the warning would indeed
not be an actual reference leak. But how could its reference show up
after 140 seconds if the bridge's creation and deletion were all within
a couple of milliseconds?
So I let the system run for a bit longer with the reproducer, and after
~200 seconds, the kernel crashed and complained that some tasks had
been waiting for too long (more than 143 seconds) trying to get hold of
the br_ioctl_mutex. This was also quite strange to me, since on the
surface it definitely looked like a deadlock, but the strict locking
order as I described previously should prevent any deadlocks from
happening.
Anyways, I decided to test switching up the lock order, since both the
refcnt warning and the task stall seemed closely related to the above
mentioned locks. When I ran the reproducer again after the patch, both
the warning and the stall issue went away. So I guess the patch is
still relevant in preventing bugs in some extreme cases -- although the
scenario created by the reproducer would probably never happen in real
usages?
Please let me know whether you have any thoughts on how the above
issues were triggered, and what other information I could gather to
further demystify this bug. Thank you again for your help!
Best regards,
Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
@ 2023-08-19 22:50 ` Ziqi Zhao
0 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-08-19 22:50 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: arnd, bridge, davem, edumazet, f.fainelli, ivan.orlov0322,
keescook, kuba, hkallweit1, mudongliangabcd, nikolay, pabeni,
roopa, skhan, syzbot+881d65229ca4f9ae8c84, vladimir.oltean,
linux-kernel, netdev, syzkaller-bugs
On Sat, Aug 19, 2023 at 12:25:15PM +0300, Nikolay Aleksandrov wrote:
Hi Nik,
Thank you so much for reviewing the patch and getting back to me!
> IIRC there was no bug, it was a false-positive. The reference is held a bit
> longer but then released, so the device is deleted later.
> If you reproduced it, is the device later removed or is it really stuck?
I ran the reproducer again without the patch and it seems you are
correct. It was trying to create a very short-lived bridge, then delete
it immediately in the next call. The device in question "wpan4" never
showed up when I polled with `ip link` in the VM, so I'd say it did not
get stuck.
> How would it leak a reference, could you elaborate?
> The reference is always taken and always released after the call.
This was where I got a bit confused too. The system had a timeout of
140 seconds for the unregister_netdevice check. If the bridge in
question was created and deleted repeatedly, the warning would indeed
not be an actual reference leak. But how could its reference show up
after 140 seconds if the bridge's creation and deletion were all within
a couple of milliseconds?
So I let the system run for a bit longer with the reproducer, and after
~200 seconds, the kernel crashed and complained that some tasks had
been waiting for too long (more than 143 seconds) trying to get hold of
the br_ioctl_mutex. This was also quite strange to me, since on the
surface it definitely looked like a deadlock, but the strict locking
order as I described previously should prevent any deadlocks from
happening.
Anyways, I decided to test switching up the lock order, since both the
refcnt warning and the task stall seemed closely related to the above
mentioned locks. When I ran the reproducer again after the patch, both
the warning and the stall issue went away. So I guess the patch is
still relevant in preventing bugs in some extreme cases -- although the
scenario created by the reproducer would probably never happen in real
usages?
Please let me know whether you have any thoughts on how the above
issues were triggered, and what other information I could gather to
further demystify this bug. Thank you again for your help!
Best regards,
Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Bridge] [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
2023-08-19 22:50 ` Ziqi Zhao
@ 2023-08-22 10:40 ` Nikolay Aleksandrov
-1 siblings, 0 replies; 23+ messages in thread
From: Nikolay Aleksandrov @ 2023-08-22 10:40 UTC (permalink / raw)
To: Ziqi Zhao
Cc: f.fainelli, ivan.orlov0322, keescook, arnd, vladimir.oltean,
bridge, syzkaller-bugs, mudongliangabcd, linux-kernel, edumazet,
netdev, nikolay, roopa, syzbot+881d65229ca4f9ae8c84, kuba, skhan,
pabeni, davem, hkallweit1
On 8/20/23 01:50, Ziqi Zhao wrote:
> On Sat, Aug 19, 2023 at 12:25:15PM +0300, Nikolay Aleksandrov wrote:
> Hi Nik,
>
> Thank you so much for reviewing the patch and getting back to me!
>
>> IIRC there was no bug, it was a false-positive. The reference is held a bit
>> longer but then released, so the device is deleted later.
>
>> If you reproduced it, is the device later removed or is it really stuck?
>
> I ran the reproducer again without the patch and it seems you are
> correct. It was trying to create a very short-lived bridge, then delete
> it immediately in the next call. The device in question "wpan4" never
> showed up when I polled with `ip link` in the VM, so I'd say it did not
> get stuck.
>
>> How would it leak a reference, could you elaborate?
>> The reference is always taken and always released after the call.
>
> This was where I got a bit confused too. The system had a timeout of
> 140 seconds for the unregister_netdevice check. If the bridge in
> question was created and deleted repeatedly, the warning would indeed
> not be an actual reference leak. But how could its reference show up
> after 140 seconds if the bridge's creation and deletion were all within
> a couple of milliseconds?
>
> So I let the system run for a bit longer with the reproducer, and after
> ~200 seconds, the kernel crashed and complained that some tasks had
> been waiting for too long (more than 143 seconds) trying to get hold of
> the br_ioctl_mutex. This was also quite strange to me, since on the
> surface it definitely looked like a deadlock, but the strict locking
> order as I described previously should prevent any deadlocks from
> happening.
>
> Anyways, I decided to test switching up the lock order, since both the
> refcnt warning and the task stall seemed closely related to the above
> mentioned locks. When I ran the reproducer again after the patch, both
> the warning and the stall issue went away. So I guess the patch is
> still relevant in preventing bugs in some extreme cases -- although the
> scenario created by the reproducer would probably never happen in real
> usages?
>
Thank you for testing, but we really need to understand what is going on
and why the device isn't getting deleted for so long. Currently I don't
have the time to debug it properly (I'll be able to next week at the
earliest). We can't apply the patch based only on tests without
understanding the underlying issue. I'd look into what
the reproducer is doing exactly and also check the system state while
the deadlock has happened. Also you can list the currently held locks
(if CONFIG_LOCKDEP is enabled) via magic sysrq + d for example. See
which process is holding them, what are their priorities and so on.
Try to build some theory of how a deadlock might happen and then go
about proving it. Does the 8021q module have the same problem? It uses
similar code to set its hook.
> Please let me know whether you have any thoughts on how the above
> issues were triggered, and what other information I could gather to
> further demystify this bug. Thank you again for your help!
>
> Best regards,
> Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
@ 2023-08-22 10:40 ` Nikolay Aleksandrov
0 siblings, 0 replies; 23+ messages in thread
From: Nikolay Aleksandrov @ 2023-08-22 10:40 UTC (permalink / raw)
To: Ziqi Zhao
Cc: arnd, bridge, davem, edumazet, f.fainelli, ivan.orlov0322,
keescook, kuba, hkallweit1, mudongliangabcd, nikolay, pabeni,
roopa, skhan, syzbot+881d65229ca4f9ae8c84, vladimir.oltean,
linux-kernel, netdev, syzkaller-bugs
On 8/20/23 01:50, Ziqi Zhao wrote:
> On Sat, Aug 19, 2023 at 12:25:15PM +0300, Nikolay Aleksandrov wrote:
> Hi Nik,
>
> Thank you so much for reviewing the patch and getting back to me!
>
>> IIRC there was no bug, it was a false-positive. The reference is held a bit
>> longer but then released, so the device is deleted later.
>
>> If you reproduced it, is the device later removed or is it really stuck?
>
> I ran the reproducer again without the patch and it seems you are
> correct. It was trying to create a very short-lived bridge, then delete
> it immediately in the next call. The device in question "wpan4" never
> showed up when I polled with `ip link` in the VM, so I'd say it did not
> get stuck.
>
>> How would it leak a reference, could you elaborate?
>> The reference is always taken and always released after the call.
>
> This was where I got a bit confused too. The system had a timeout of
> 140 seconds for the unregister_netdevice check. If the bridge in
> question was created and deleted repeatedly, the warning would indeed
> not be an actual reference leak. But how could its reference show up
> after 140 seconds if the bridge's creation and deletion were all within
> a couple of milliseconds?
>
> So I let the system run for a bit longer with the reproducer, and after
> ~200 seconds, the kernel crashed and complained that some tasks had
> been waiting for too long (more than 143 seconds) trying to get hold of
> the br_ioctl_mutex. This was also quite strange to me, since on the
> surface it definitely looked like a deadlock, but the strict locking
> order as I described previously should prevent any deadlocks from
> happening.
>
> Anyways, I decided to test switching up the lock order, since both the
> refcnt warning and the task stall seemed closely related to the above
> mentioned locks. When I ran the reproducer again after the patch, both
> the warning and the stall issue went away. So I guess the patch is
> still relevant in preventing bugs in some extreme cases -- although the
> scenario created by the reproducer would probably never happen in real
> usages?
>
Thank you for testing, but we really need to understand what is going on
and why the device isn't getting deleted for so long. Currently I don't
have the time to debug it properly (I'll be able to next week at the
earliest). We can't apply the patch based only on tests without
understanding the underlying issue. I'd look into what
the reproducer is doing exactly and also check the system state while
the deadlock has happened. Also you can list the currently held locks
(if CONFIG_LOCKDEP is enabled) via magic sysrq + d for example. See
which process is holding them, what are their priorities and so on.
Try to build some theory of how a deadlock might happen and then go
about proving it. Does the 8021q module have the same problem? It uses
similar code to set its hook.
> Please let me know whether you have any thoughts on how the above
> issues were triggered, and what other information I could gather to
> further demystify this bug. Thank you again for your help!
>
> Best regards,
> Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Bridge] [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
2023-08-22 10:40 ` Nikolay Aleksandrov
@ 2023-08-23 9:38 ` Ziqi Zhao
-1 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-08-23 9:38 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: f.fainelli, ivan.orlov0322, keescook, arnd, vladimir.oltean,
bridge, syzkaller-bugs, mudongliangabcd, linux-kernel, edumazet,
netdev, nikolay, roopa, syzbot+881d65229ca4f9ae8c84, kuba, skhan,
pabeni, davem, hkallweit1
On Tue, Aug 22, 2023 at 01:40:45PM +0300, Nikolay Aleksandrov wrote:
> Thank you for testing, but we really need to understand what is going on and
> why the device isn't getting deleted for so long. Currently I don't have the
> time to debug it properly (I'll be able to next week at the earliest). We
> can't apply the patch based only on tests without understanding the
> underlying issue. I'd look into what
> the reproducer is doing exactly and also check the system state while the
> deadlock has happened. Also you can list the currently held locks (if
> CONFIG_LOCKDEP is enabled) via magic sysrq + d for example. See which
> process is holding them, what are their priorities and so on.
> Try to build some theory of how a deadlock might happen and then go
> about proving it. Does the 8021q module have the same problem? It uses
> similar code to set its hook.
Hi Nik,
Thank you so much for the instructions! I was able to obtain a decoded
stacktrace showing the reproducer behavior in my QEMU VM running kernel
6.5-rc4, in case that would give us more context for pinpointing the
problem. Here's a link to the output:
https://pastecat.io/?p=IlKZlflN9j2Z2mspjKe7
Basically, after running the reproducer (line 1854) for about 180
seconnds or so, the unregister_netdevice warning was shown (line 1856),
and then after another 50 seconds, the kernel detected that some tasks
have been stalled for more than 143 seconds (line 1866), so it panicked
on the blocked tasks (line 2116). Before the panic, we did get to see
all the locks held in the system (line 2068), and it did show that many
processes created by the reproducer were contending the br_ioctl_mutex.
I'm now starting to wonder whether this is really a deadlock, or simply
some tasks not being able to grab the lock because so many processes
are trying to acquire it.
Let me know what you think about the situation shown in the above log,
and let's keep in touch for any future debugging. Thank you again for
guiding me through the problem!
Best regards,
Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
@ 2023-08-23 9:38 ` Ziqi Zhao
0 siblings, 0 replies; 23+ messages in thread
From: Ziqi Zhao @ 2023-08-23 9:38 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: arnd, bridge, davem, edumazet, f.fainelli, ivan.orlov0322,
keescook, kuba, hkallweit1, mudongliangabcd, nikolay, pabeni,
roopa, skhan, syzbot+881d65229ca4f9ae8c84, vladimir.oltean,
linux-kernel, netdev, syzkaller-bugs
On Tue, Aug 22, 2023 at 01:40:45PM +0300, Nikolay Aleksandrov wrote:
> Thank you for testing, but we really need to understand what is going on and
> why the device isn't getting deleted for so long. Currently I don't have the
> time to debug it properly (I'll be able to next week at the earliest). We
> can't apply the patch based only on tests without understanding the
> underlying issue. I'd look into what
> the reproducer is doing exactly and also check the system state while the
> deadlock has happened. Also you can list the currently held locks (if
> CONFIG_LOCKDEP is enabled) via magic sysrq + d for example. See which
> process is holding them, what are their priorities and so on.
> Try to build some theory of how a deadlock might happen and then go
> about proving it. Does the 8021q module have the same problem? It uses
> similar code to set its hook.
Hi Nik,
Thank you so much for the instructions! I was able to obtain a decoded
stacktrace showing the reproducer behavior in my QEMU VM running kernel
6.5-rc4, in case that would give us more context for pinpointing the
problem. Here's a link to the output:
https://pastecat.io/?p=IlKZlflN9j2Z2mspjKe7
Basically, after running the reproducer (line 1854) for about 180
seconnds or so, the unregister_netdevice warning was shown (line 1856),
and then after another 50 seconds, the kernel detected that some tasks
have been stalled for more than 143 seconds (line 1866), so it panicked
on the blocked tasks (line 2116). Before the panic, we did get to see
all the locks held in the system (line 2068), and it did show that many
processes created by the reproducer were contending the br_ioctl_mutex.
I'm now starting to wonder whether this is really a deadlock, or simply
some tasks not being able to grab the lock because so many processes
are trying to acquire it.
Let me know what you think about the situation shown in the above log,
and let's keep in touch for any future debugging. Thank you again for
guiding me through the problem!
Best regards,
Ziqi
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Bridge] [PATCH] net: bridge: Fix refcnt issues in dev_ioctl
2023-08-23 9:38 ` Ziqi Zhao
(?)
@ 2024-02-12 15:28 ` Alexander Ofitserov
-1 siblings, 0 replies; 23+ messages in thread
From: Alexander Ofitserov @ 2024-02-12 15:28 UTC (permalink / raw)
To: astrajoan
Cc: arnd, bridge, davem, edumazet, f.fainelli, hkallweit1,
ivan.orlov0322, keescook, kuba, linux-kernel, mudongliangabcd,
netdev, nikolay, pabeni, razor, roopa, skhan,
syzbot+881d65229ca4f9ae8c84, syzkaller-bugs, vladimir.oltean,
dutyrok, Alexander Ofitserov
On Wed, Aug 23, 2023 at 00:38:46PM +0300, Ziqi Zhao wrote:
> On Tue, Aug 22, 2023 at 01:40:45PM +0300, Nikolay Aleksandrov wrote:
> > Thank you for testing, but we really need to understand what is going on
> > and why the device isn't getting deleted for so long. Currently I don't
> > have the time to debug it properly (I'll be able to next week at the
> > earliest). We can't apply the patch based only on tests without
> > understanding the underlying issue. I'd look into what
> > the reproducer is doing exactly and also check the system state while the
> > deadlock has happened. Also you can list the currently held locks (if
> > CONFIG_LOCKDEP is enabled) via magic sysrq + d for example. See which
> > process is holding them, what are their priorities and so on.
> > Try to build some theory of how a deadlock might happen and then go
> > about proving it. Does the 8021q module have the same problem? It uses
> > similar code to set its hook.
>
> Hi Nik,
>
> Thank you so much for the instructions! I was able to obtain a decoded
> stacktrace showing the reproducer behavior in my QEMU VM running kernel
> 6.5-rc4, in case that would give us more context for pinpointing the
> problem. Here's a link to the output:
>
> https://pastecat.io/?p=IlKZlflN9j2Z2mspjKe7
>
> Basically, after running the reproducer (line 1854) for about 180
> seconnds or so, the unregister_netdevice warning was shown (line 1856),
> and then after another 50 seconds, the kernel detected that some tasks
> have been stalled for more than 143 seconds (line 1866), so it panicked
> on the blocked tasks (line 2116). Before the panic, we did get to see
> all the locks held in the system (line 2068), and it did show that many
> processes created by the reproducer were contending the br_ioctl_mutex.
> I'm now starting to wonder whether this is really a deadlock, or simply
> some tasks not being able to grab the lock because so many processes
> are trying to acquire it.
>
> Let me know what you think about the situation shown in the above log,
> and let's keep in touch for any future debugging. Thank you again for
> guiding me through the problem!
>
> Best regards,
> Ziqi
Hello,
I've also encountered this bug while fuzzing. Is there any going work on this
bug?
--
2.42.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
2023-06-10 1:34 ` syzbot
` (2 preceding siblings ...)
(?)
@ 2025-08-25 12:35 ` Hillf Danton
2025-08-25 13:51 ` syzbot
-1 siblings, 1 reply; 23+ messages in thread
From: Hillf Danton @ 2025-08-25 12:35 UTC (permalink / raw)
To: syzbot; +Cc: linux-kernel, syzkaller-bugs
> Date: Fri, 09 Jun 2023 18:34:58 -0700 [thread overview]
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 67faabbde36b selftests/bpf: Add missing prototypes for sev..
> git tree: bpf-next
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=1381363b280000
> kernel config: https://syzkaller.appspot.com/x/.config?x=5335204dcdecfda
> dashboard link: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=132faf93280000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10532add280000
#syz test upstream master
--- a/net/can/j1939/j1939-priv.h
+++ b/net/can/j1939/j1939-priv.h
@@ -59,6 +59,7 @@ struct j1939_priv {
/* segments need a lock to protect the above list */
rwlock_t lock;
+ int unregistered;
struct net_device *ndev;
/* list of 256 ecu ptrs, that cache the claimed addresses.
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -157,13 +157,15 @@ static void __j1939_priv_release(struct
struct j1939_priv *priv = container_of(kref, struct j1939_priv, kref);
struct net_device *ndev = priv->ndev;
- netdev_dbg(priv->ndev, "%s: 0x%p\n", __func__, priv);
+ if (!priv->unregistered)
+ netdev_dbg(priv->ndev, "%s: 0x%p\n", __func__, priv);
WARN_ON_ONCE(!list_empty(&priv->active_session_list));
WARN_ON_ONCE(!list_empty(&priv->ecus));
WARN_ON_ONCE(!list_empty(&priv->j1939_socks));
- dev_put(ndev);
+ if (!priv->unregistered)
+ dev_put(ndev);
kfree(priv);
}
@@ -377,6 +379,10 @@ static int j1939_netdev_notify(struct no
j1939_sk_netdev_event_netdown(priv);
j1939_ecu_unmap_all(priv);
break;
+ case NETDEV_UNREGISTER:
+ priv->unregistered++;
+ dev_put(priv->ndev);
+ break;
}
j1939_priv_put(priv);
--
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
2025-08-25 12:35 ` [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8) Hillf Danton
@ 2025-08-25 13:51 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2025-08-25 13:51 UTC (permalink / raw)
To: hdanton, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: use-after-free Read in j1939_netdev_stop
==================================================================
BUG: KASAN: use-after-free in read_pnet include/net/net_namespace.h:409 [inline]
BUG: KASAN: use-after-free in dev_net include/linux/netdevice.h:2718 [inline]
BUG: KASAN: use-after-free in j1939_can_rx_unregister net/can/j1939/main.c:202 [inline]
BUG: KASAN: use-after-free in __j1939_rx_release net/can/j1939/main.c:218 [inline]
BUG: KASAN: use-after-free in kref_put_mutex include/linux/kref.h:86 [inline]
BUG: KASAN: use-after-free in j1939_netdev_stop+0x2ab/0x2d0 net/can/j1939/main.c:311
Read of size 8 at addr ffff888023f80108 by task syz.0.17/6524
CPU: 0 UID: 0 PID: 6524 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2025
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:378 [inline]
print_report+0xcd/0x630 mm/kasan/report.c:482
kasan_report+0xe0/0x110 mm/kasan/report.c:595
read_pnet include/net/net_namespace.h:409 [inline]
dev_net include/linux/netdevice.h:2718 [inline]
j1939_can_rx_unregister net/can/j1939/main.c:202 [inline]
__j1939_rx_release net/can/j1939/main.c:218 [inline]
kref_put_mutex include/linux/kref.h:86 [inline]
j1939_netdev_stop+0x2ab/0x2d0 net/can/j1939/main.c:311
j1939_sk_release+0x5c3/0x8e0 net/can/j1939/socket.c:651
__sock_release+0xb3/0x270 net/socket.c:649
sock_close+0x1c/0x30 net/socket.c:1439
__fput+0x402/0xb70 fs/file_table.c:468
task_work_run+0x14d/0x240 kernel/task_work.c:227
exit_task_work include/linux/task_work.h:40 [inline]
do_exit+0x86f/0x2bf0 kernel/exit.c:961
do_group_exit+0xd3/0x2a0 kernel/exit.c:1102
get_signal+0x2673/0x26d0 kernel/signal.c:3034
arch_do_signal_or_restart+0x8f/0x7d0 arch/x86/kernel/signal.c:337
exit_to_user_mode_loop+0x84/0x110 kernel/entry/common.c:40
exit_to_user_mode_prepare include/linux/irq-entry-common.h:225 [inline]
syscall_exit_to_user_mode_work include/linux/entry-common.h:175 [inline]
syscall_exit_to_user_mode include/linux/entry-common.h:210 [inline]
do_syscall_64+0x3f6/0x4c0 arch/x86/entry/syscall_64.c:100
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f408338ebe9
Code: Unable to access opcode bytes at 0x7f408338ebbf.
RSP: 002b:00007f4084235038 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: 0000000000000024 RBX: 00007f40835b5fa0 RCX: 00007f408338ebe9
RDX: 0000000000000000 RSI: 0000200000000200 RDI: 0000000000000003
RBP: 00007f4083411e19 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f40835b6038 R14: 00007f40835b5fa0 R15: 00007ffd085a2ed8
</TASK>
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888023f80000 pfn:0x23f80
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000000000 ffffea000174f608 ffffea0000c2c208 0000000000000000
raw: ffff888023f80000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as freed
page last allocated via order 3, migratetype Unmovable, gfp_mask 0x446dc0(GFP_KERNEL_ACCOUNT|__GFP_ZERO|__GFP_NOWARN|__GFP_RETRY_MAYFAIL|__GFP_COMP), pid 6440, tgid 6440 (syz-executor), ts 122667010586, free_ts 125938931446
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1c0/0x230 mm/page_alloc.c:1851
prep_new_page mm/page_alloc.c:1859 [inline]
get_page_from_freelist+0x132b/0x38e0 mm/page_alloc.c:3858
__alloc_frozen_pages_noprof+0x261/0x23f0 mm/page_alloc.c:5148
alloc_pages_mpol+0x1fb/0x550 mm/mempolicy.c:2416
___kmalloc_large_node+0xed/0x160 mm/slub.c:4306
__kmalloc_large_node_noprof+0x1c/0x70 mm/slub.c:4337
__do_kmalloc_node mm/slub.c:4353 [inline]
__kvmalloc_node_noprof.cold+0xb/0x65 mm/slub.c:5052
alloc_netdev_mqs+0xd2/0x1530 net/core/dev.c:11812
rtnl_create_link+0xc08/0xf90 net/core/rtnetlink.c:3633
vxcan_newlink+0x2f8/0x640 drivers/net/can/vxcan.c:208
rtnl_newlink_create net/core/rtnetlink.c:3825 [inline]
__rtnl_newlink net/core/rtnetlink.c:3942 [inline]
rtnl_newlink+0xc45/0x2000 net/core/rtnetlink.c:4057
rtnetlink_rcv_msg+0x95e/0xe90 net/core/rtnetlink.c:6946
netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
sock_sendmsg_nosec net/socket.c:714 [inline]
__sock_sendmsg net/socket.c:729 [inline]
__sys_sendto+0x4a3/0x520 net/socket.c:2228
page last free pid 6524 tgid 6523 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1395 [inline]
__free_frozen_pages+0x7d5/0x10f0 mm/page_alloc.c:2895
device_release+0xa1/0x240 drivers/base/core.c:2565
kobject_cleanup lib/kobject.c:689 [inline]
kobject_release lib/kobject.c:720 [inline]
kref_put include/linux/kref.h:65 [inline]
kobject_put+0x1e7/0x5a0 lib/kobject.c:737
netdev_run_todo+0x7e9/0x1320 net/core/dev.c:11513
rtnl_unlock net/core/rtnetlink.c:157 [inline]
rtnl_net_unlock include/linux/rtnetlink.h:135 [inline]
rtnl_dellink+0x3da/0xa80 net/core/rtnetlink.c:3563
rtnetlink_rcv_msg+0x95e/0xe90 net/core/rtnetlink.c:6946
netlink_rcv_skb+0x158/0x420 net/netlink/af_netlink.c:2552
netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
netlink_unicast+0x5a7/0x870 net/netlink/af_netlink.c:1346
netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
sock_sendmsg_nosec net/socket.c:714 [inline]
__sock_sendmsg net/socket.c:729 [inline]
____sys_sendmsg+0xa95/0xc70 net/socket.c:2614
___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
__sys_sendmsg+0x16d/0x220 net/socket.c:2700
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Memory state around the buggy address:
ffff888023f80000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888023f80080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff888023f80100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff888023f80180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888023f80200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
Tested on:
commit: 1b237f19 Linux 6.17-rc3
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10b66862580000
kernel config: https://syzkaller.appspot.com/x/.config?x=d4703ac89d9e185a
dashboard link: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1417b862580000
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
2023-06-10 1:34 ` syzbot
` (3 preceding siblings ...)
(?)
@ 2025-08-26 1:50 ` Hillf Danton
2025-08-26 2:48 ` syzbot
-1 siblings, 1 reply; 23+ messages in thread
From: Hillf Danton @ 2025-08-26 1:50 UTC (permalink / raw)
To: syzbot; +Cc: linux-kernel, syzkaller-bugs
> Date: Fri, 09 Jun 2023 18:34:58 -0700 [thread overview]
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 67faabbde36b selftests/bpf: Add missing prototypes for sev..
> git tree: bpf-next
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=1381363b280000
> kernel config: https://syzkaller.appspot.com/x/.config?x=5335204dcdecfda
> dashboard link: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=132faf93280000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10532add280000
#syz test upstream master
--- a/net/can/j1939/j1939-priv.h
+++ b/net/can/j1939/j1939-priv.h
@@ -59,6 +59,7 @@ struct j1939_priv {
/* segments need a lock to protect the above list */
rwlock_t lock;
+ int unregistered;
struct net_device *ndev;
/* list of 256 ecu ptrs, that cache the claimed addresses.
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -157,13 +157,15 @@ static void __j1939_priv_release(struct
struct j1939_priv *priv = container_of(kref, struct j1939_priv, kref);
struct net_device *ndev = priv->ndev;
- netdev_dbg(priv->ndev, "%s: 0x%p\n", __func__, priv);
+ if (!priv->unregistered)
+ netdev_dbg(priv->ndev, "%s: 0x%p\n", __func__, priv);
WARN_ON_ONCE(!list_empty(&priv->active_session_list));
WARN_ON_ONCE(!list_empty(&priv->ecus));
WARN_ON_ONCE(!list_empty(&priv->j1939_socks));
- dev_put(ndev);
+ if (!priv->unregistered)
+ dev_put(ndev);
kfree(priv);
}
@@ -197,7 +199,8 @@ static void j1939_can_rx_unregister(stru
{
struct net_device *ndev = priv->ndev;
- can_rx_unregister(dev_net(ndev), ndev, J1939_CAN_ID, J1939_CAN_MASK,
+ if (!priv->unregistered)
+ can_rx_unregister(dev_net(ndev), ndev, J1939_CAN_ID, J1939_CAN_MASK,
j1939_can_recv, priv);
/* The last reference of priv is dropped by the RCU deferred
@@ -377,6 +380,10 @@ static int j1939_netdev_notify(struct no
j1939_sk_netdev_event_netdown(priv);
j1939_ecu_unmap_all(priv);
break;
+ case NETDEV_UNREGISTER:
+ priv->unregistered++;
+ dev_put(priv->ndev);
+ break;
}
j1939_priv_put(priv);
--
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [syzbot] [net?] unregister_netdevice: waiting for DEV to become free (8)
2025-08-26 1:50 ` Hillf Danton
@ 2025-08-26 2:48 ` syzbot
0 siblings, 0 replies; 23+ messages in thread
From: syzbot @ 2025-08-26 2:48 UTC (permalink / raw)
To: hdanton, linux-kernel, syzkaller-bugs
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
KASAN: use-after-free Read in j1939_netdev_stop
==================================================================
BUG: KASAN: use-after-free in netdev_get_ml_priv include/linux/netdevice.h:2692 [inline]
BUG: KASAN: use-after-free in can_get_ml_priv include/linux/can/can-ml.h:71 [inline]
BUG: KASAN: use-after-free in j1939_priv_set net/can/j1939/main.c:150 [inline]
BUG: KASAN: use-after-free in __j1939_rx_release net/can/j1939/main.c:221 [inline]
BUG: KASAN: use-after-free in kref_put_mutex include/linux/kref.h:86 [inline]
BUG: KASAN: use-after-free in j1939_netdev_stop+0x2df/0x320 net/can/j1939/main.c:312
Read of size 4 at addr ffff888062a506b0 by task syz.0.17/6467
CPU: 0 UID: 0 PID: 6467 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/12/2025
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x116/0x1f0 lib/dump_stack.c:120
print_address_description mm/kasan/report.c:378 [inline]
print_report+0xcd/0x630 mm/kasan/report.c:482
kasan_report+0xe0/0x110 mm/kasan/report.c:595
netdev_get_ml_priv include/linux/netdevice.h:2692 [inline]
can_get_ml_priv include/linux/can/can-ml.h:71 [inline]
j1939_priv_set net/can/j1939/main.c:150 [inline]
__j1939_rx_release net/can/j1939/main.c:221 [inline]
kref_put_mutex include/linux/kref.h:86 [inline]
j1939_netdev_stop+0x2df/0x320 net/can/j1939/main.c:312
j1939_sk_release+0x5c3/0x8e0 net/can/j1939/socket.c:651
__sock_release+0xb0/0x270 net/socket.c:649
sock_close+0x1c/0x30 net/socket.c:1439
__fput+0x402/0xb70 fs/file_table.c:468
task_work_run+0x14d/0x240 kernel/task_work.c:227
exit_task_work include/linux/task_work.h:40 [inline]
do_exit+0x86f/0x2bf0 kernel/exit.c:961
do_group_exit+0xd3/0x2a0 kernel/exit.c:1102
get_signal+0x2673/0x26d0 kernel/signal.c:3034
arch_do_signal_or_restart+0x8f/0x7d0 arch/x86/kernel/signal.c:337
exit_to_user_mode_loop+0x84/0x110 kernel/entry/common.c:40
exit_to_user_mode_prepare include/linux/irq-entry-common.h:225 [inline]
syscall_exit_to_user_mode_work include/linux/entry-common.h:175 [inline]
syscall_exit_to_user_mode include/linux/entry-common.h:210 [inline]
do_syscall_64+0x3f6/0x4c0 arch/x86/entry/syscall_64.c:100
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f15b7b8ebe9
Code: Unable to access opcode bytes at 0x7f15b7b8ebbf.
RSP: 002b:00007f15b899d038 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: 0000000000000024 RBX: 00007f15b7db5fa0 RCX: 00007f15b7b8ebe9
RDX: 0000000000000000 RSI: 0000200000000200 RDI: 0000000000000003
RBP: 00007f15b7c11e19 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f15b7db6038 R14: 00007f15b7db5fa0 R15: 00007ffec48e9178
</TASK>
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x62a50
flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
raw: 00fff00000000000 ffffea0001db7008 ffffea00018a9208 0000000000000000
raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as freed
page last allocated via order 3, migratetype Unmovable, gfp_mask 0x446dc0(GFP_KERNEL_ACCOUNT|__GFP_ZERO|__GFP_NOWARN|__GFP_RETRY_MAYFAIL|__GFP_COMP), pid 6357, tgid 6357 (syz-executor), ts 118494889601, free_ts 123112554125
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x1c0/0x230 mm/page_alloc.c:1851
prep_new_page mm/page_alloc.c:1859 [inline]
get_page_from_freelist+0x132b/0x38e0 mm/page_alloc.c:3858
__alloc_frozen_pages_noprof+0x261/0x23f0 mm/page_alloc.c:5148
alloc_pages_mpol+0x1fb/0x550 mm/mempolicy.c:2416
___kmalloc_large_node+0xed/0x160 mm/slub.c:4306
__kmalloc_large_node_noprof+0x1c/0x70 mm/slub.c:4337
__do_kmalloc_node mm/slub.c:4353 [inline]
__kvmalloc_node_noprof.cold+0xb/0x65 mm/slub.c:5052
alloc_netdev_mqs+0xd2/0x1530 net/core/dev.c:11812
rtnl_create_link+0xc08/0xf90 net/core/rtnetlink.c:3633
vxcan_newlink+0x2f8/0x640 drivers/net/can/vxcan.c:208
rtnl_newlink_create net/core/rtnetlink.c:3825 [inline]
__rtnl_newlink net/core/rtnetlink.c:3942 [inline]
rtnl_newlink+0xc45/0x2000 net/core/rtnetlink.c:4057
rtnetlink_rcv_msg+0x95b/0xe90 net/core/rtnetlink.c:6946
netlink_rcv_skb+0x155/0x420 net/netlink/af_netlink.c:2552
netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1346
netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
sock_sendmsg_nosec net/socket.c:714 [inline]
__sock_sendmsg net/socket.c:729 [inline]
__sys_sendto+0x4a0/0x520 net/socket.c:2228
page last free pid 6467 tgid 6466 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
free_pages_prepare mm/page_alloc.c:1395 [inline]
__free_frozen_pages+0x7d5/0x10f0 mm/page_alloc.c:2895
device_release+0xa1/0x240 drivers/base/core.c:2565
kobject_cleanup lib/kobject.c:689 [inline]
kobject_release lib/kobject.c:720 [inline]
kref_put include/linux/kref.h:65 [inline]
kobject_put+0x1e7/0x5a0 lib/kobject.c:737
netdev_run_todo+0x7e9/0x1320 net/core/dev.c:11513
rtnl_unlock net/core/rtnetlink.c:157 [inline]
rtnl_net_unlock include/linux/rtnetlink.h:135 [inline]
rtnl_dellink+0x3da/0xa80 net/core/rtnetlink.c:3563
rtnetlink_rcv_msg+0x95b/0xe90 net/core/rtnetlink.c:6946
netlink_rcv_skb+0x155/0x420 net/netlink/af_netlink.c:2552
netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
netlink_unicast+0x5aa/0x870 net/netlink/af_netlink.c:1346
netlink_sendmsg+0x8d1/0xdd0 net/netlink/af_netlink.c:1896
sock_sendmsg_nosec net/socket.c:714 [inline]
__sock_sendmsg net/socket.c:729 [inline]
____sys_sendmsg+0xa98/0xc70 net/socket.c:2614
___sys_sendmsg+0x134/0x1d0 net/socket.c:2668
__sys_sendmsg+0x16d/0x220 net/socket.c:2700
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xcd/0x4c0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Memory state around the buggy address:
ffff888062a50580: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888062a50600: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff888062a50680: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff888062a50700: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff888062a50780: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
Tested on:
commit: fab1beda Merge tag 'devicetree-fixes-for-6.17-1' of gi..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16049c42580000
kernel config: https://syzkaller.appspot.com/x/.config?x=d4703ac89d9e185a
dashboard link: https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=142de862580000
^ permalink raw reply [flat|nested] 23+ messages in thread