From: Dan Carpenter <dan.carpenter@linaro.org>
To: Sam Sun <samsun1006219@gmail.com>
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-hams@vger.kernel.org, pabeni@redhat.com, kuba@kernel.org,
jreuter@yaina.de, davem@davemloft.net,
Eric Dumazet <edumazet@google.com>,
syzkaller-bugs@googlegroups.com, xrivendell7@gmail.com
Subject: Re: [Linux kernel bug] WARNING in ax25_dev_device_down
Date: Thu, 11 Apr 2024 09:31:31 +0300 [thread overview]
Message-ID: <26760fb7-a2d8-4cb5-9c47-3f91016a9a7a@moroto.mountain> (raw)
In-Reply-To: <CAEkJfYNYe5tSMfAn9K_zJ1O_Vu7jxJDZv_uYCgTW=NZiUzcAuw@mail.gmail.com>
On Tue, Apr 09, 2024 at 10:02:49PM +0800, Sam Sun wrote:
> Dear developers and maintainers,
>
> We encountered a kernel warning in function ax25_dev_device_down
> during testing using our modified syzkaller. It is tested against the
> latest upstream linux (6.9-rc3). C repro and kernel config are
> attached to this email. Kernel log is listed below.
> ```
> WARNING: CPU: 0 PID: 8121 at lib/ref_tracker.c:255
> ref_tracker_free+0x610/0x830 lib/ref_tracker.c:255
> Modules linked in:
> CPU: 0 PID: 8121 Comm: syz-executor329 Not tainted 6.7.0-rc7 #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> RIP: 0010:ref_tracker_free+0x610/0x830 lib/ref_tracker.c:255
> Code: 00 00 44 8b 73 18 31 ff 44 89 f6 e8 7a 58 fe fc 45 85 f6 0f 85
> a7 00 00 00 e8 4c 5c fe fc 4c 89 ee 48 89 ef e8 11 0a e5 05 90 <0f> 0b
> 90 41 bd ea ff ff ff e9 51 fd ff ff e8 2d 5c fe fc 4c 8d 75
> RSP: 0018:ffffc900029bf8b8 EFLAGS: 00010286
> RAX: 0000000080000000 RBX: ffff888106185480 RCX: 0000000000000000
> RDX: 0000000000000001 RSI: 0000000000000001 RDI: 0000000000000001
> RBP: ffff8881136c85b8 R08: 0000000000000001 R09: fffffbfff23e8bd1
> R10: 0000000000000001 R11: 0000000000000001 R12: 1ffff92000537f19
> R13: 0000000000000292 R14: 00000000067a01d1 R15: ffff888106185498
> FS: 0000555556e663c0(0000) GS:ffff888063a00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fc148041ba8 CR3: 0000000016374000 CR4: 0000000000750ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
> <TASK>
> netdev_tracker_free include/linux/netdevice.h:4127 [inline]
> netdev_put include/linux/netdevice.h:4144 [inline]
> netdev_put include/linux/netdevice.h:4140 [inline]
> ax25_dev_device_down+0x2bc/0x420 net/ax25/ax25_dev.c:140
^^^^
The locking in ax25_dev_device_down() seems pretty suspect.
It takes:
ax25_dev = ax25_dev_ax25dev()
then it takes the &ax25_dev_lock, then it drops the lock, then on line
140 mentioned in this stack trace it calls:
netdev_put(dev, &ax25_dev->dev_tracker);
That can race with ax25_dev_free() which has proper locking. The
temptation is to do something like this, but I don't know this code well
and can't test it.
regards,
dan carpenter
diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c
index 282ec581c072..68c2945d6051 100644
--- a/net/ax25/ax25_dev.c
+++ b/net/ax25/ax25_dev.c
@@ -97,13 +97,15 @@ void ax25_dev_device_down(struct net_device *dev)
{
ax25_dev *s, *ax25_dev;
- if ((ax25_dev = ax25_dev_ax25dev(dev)) == NULL)
+ spin_lock_bh(&ax25_dev_lock);
+
+ if ((ax25_dev = ax25_dev_ax25dev(dev)) == NULL) {
+ spin_unlock_bh(&ax25_dev_lock);
return;
+ }
ax25_unregister_dev_sysctl(ax25_dev);
- spin_lock_bh(&ax25_dev_lock);
-
#ifdef CONFIG_AX25_DAMA_SLAVE
timer_shutdown_sync(&ax25_dev->dama.slave_timer);
#endif
@@ -128,17 +130,17 @@ void ax25_dev_device_down(struct net_device *dev)
s = s->next;
}
- spin_unlock_bh(&ax25_dev_lock);
dev->ax25_ptr = NULL;
ax25_dev_put(ax25_dev);
+ spin_unlock_bh(&ax25_dev_lock);
return;
unlock_put:
- spin_unlock_bh(&ax25_dev_lock);
ax25_dev_put(ax25_dev);
dev->ax25_ptr = NULL;
netdev_put(dev, &ax25_dev->dev_tracker);
ax25_dev_put(ax25_dev);
+ spin_unlock_bh(&ax25_dev_lock);
}
int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd)
prev parent reply other threads:[~2024-04-11 6:31 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 14:02 [Linux kernel bug] WARNING in ax25_dev_device_down Sam Sun
2024-04-11 6:31 ` Dan Carpenter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=26760fb7-a2d8-4cb5-9c47-3f91016a9a7a@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jreuter@yaina.de \
--cc=kuba@kernel.org \
--cc=linux-hams@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=samsun1006219@gmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=xrivendell7@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.