* [syzbot] general protection fault in l2cap_chan_timeout (2)
@ 2021-05-31 7:19 syzbot
[not found] ` <20210531090414.2558-1-hdanton@sina.com>
0 siblings, 1 reply; 3+ messages in thread
From: syzbot @ 2021-05-31 7:19 UTC (permalink / raw)
To: davem, johan.hedberg, kuba, linux-bluetooth, linux-kernel,
luiz.dentz, marcel, netdev, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: ad9f25d3 Merge tag 'netfs-lib-fixes-20200525' of git://git..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=173d383dd00000
kernel config: https://syzkaller.appspot.com/x/.config?x=266cda122a0b56c
dashboard link: https://syzkaller.appspot.com/bug?extid=008cdbf7a9044c2c2f99
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+008cdbf7a9044c2c2f99@syzkaller.appspotmail.com
general protection fault, probably for non-canonical address 0xdffffc000000005a: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x00000000000002d0-0x00000000000002d7]
CPU: 0 PID: 8 Comm: kworker/0:2 Not tainted 5.13.0-rc3-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events l2cap_chan_timeout
RIP: 0010:__mutex_lock_common kernel/locking/mutex.c:941 [inline]
RIP: 0010:__mutex_lock+0xf6/0x10c0 kernel/locking/mutex.c:1104
Code: d0 7c 08 84 d2 0f 85 cc 0c 00 00 8b 15 e3 55 5f 07 85 d2 75 29 48 8d 7d 60 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 db 0e 00 00 48 3b 6d 60 0f 85 5a 0a 00 00 bf 01
RSP: 0018:ffffc90000cd7b78 EFLAGS: 00010216
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000001
RDX: 000000000000005a RSI: 0000000000000000 RDI: 00000000000002d0
RBP: 0000000000000270 R08: ffffffff880a40d9 R09: 0000000000000000
R10: ffffffff814b4be0 R11: 0000000000000000 R12: 0000000000000000
R13: dffffc0000000000 R14: ffff888072e47020 R15: ffff8880b9c34a40
FS: 0000000000000000(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fcac15f1d58 CR3: 00000000628fa000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
Call Trace:
l2cap_chan_timeout+0x69/0x2f0 net/bluetooth/l2cap_core.c:422
process_one_work+0x98d/0x1600 kernel/workqueue.c:2276
worker_thread+0x64c/0x1120 kernel/workqueue.c:2422
kthread+0x3b1/0x4a0 kernel/kthread.c:313
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
Modules linked in:
---[ end trace d3dc393d48928266 ]---
RIP: 0010:__mutex_lock_common kernel/locking/mutex.c:941 [inline]
RIP: 0010:__mutex_lock+0xf6/0x10c0 kernel/locking/mutex.c:1104
Code: d0 7c 08 84 d2 0f 85 cc 0c 00 00 8b 15 e3 55 5f 07 85 d2 75 29 48 8d 7d 60 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 db 0e 00 00 48 3b 6d 60 0f 85 5a 0a 00 00 bf 01
RSP: 0018:ffffc90000cd7b78 EFLAGS: 00010216
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000001
RDX: 000000000000005a RSI: 0000000000000000 RDI: 00000000000002d0
RBP: 0000000000000270 R08: ffffffff880a40d9 R09: 0000000000000000
R10: ffffffff814b4be0 R11: 0000000000000000 R12: 0000000000000000
R13: dffffc0000000000 R14: ffff888072e47020 R15: ffff8880b9c34a40
FS: 0000000000000000(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b33621000 CR3: 00000000628fa000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
---
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [syzbot] general protection fault in l2cap_chan_timeout (2)
[not found] ` <20210531090414.2558-1-hdanton@sina.com>
@ 2021-06-01 2:11 ` Luiz Augusto von Dentz
[not found] ` <20210601075417.2763-1-hdanton@sina.com>
0 siblings, 1 reply; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-06-01 2:11 UTC (permalink / raw)
To: Hillf Danton
Cc: syzbot, David Miller, Johan Hedberg, Jakub Kicinski,
linux-bluetooth, Linux Kernel Mailing List, Marcel Holtmann,
open list:NETWORKING [GENERAL],
syzkaller-bugs
Hi,
On Mon, May 31, 2021 at 2:04 AM Hillf Danton <hdanton@sina.com> wrote:
>
> On Mon, 31 May 2021 00:19:17 -0700
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: ad9f25d3 Merge tag 'netfs-lib-fixes-20200525' of git://git..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=173d383dd00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=266cda122a0b56c
> > dashboard link: https://syzkaller.appspot.com/bug?extid=008cdbf7a9044c2c2f99
> >
> > Unfortunately, I don't have any reproducer for this issue yet.
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+008cdbf7a9044c2c2f99@syzkaller.appspotmail.com
> >
> > general protection fault, probably for non-canonical address 0xdffffc000000005a: 0000 [#1] PREEMPT SMP KASAN
> > KASAN: null-ptr-deref in range [0x00000000000002d0-0x00000000000002d7]
> > CPU: 0 PID: 8 Comm: kworker/0:2 Not tainted 5.13.0-rc3-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > Workqueue: events l2cap_chan_timeout
> > RIP: 0010:__mutex_lock_common kernel/locking/mutex.c:941 [inline]
> > RIP: 0010:__mutex_lock+0xf6/0x10c0 kernel/locking/mutex.c:1104
> > Code: d0 7c 08 84 d2 0f 85 cc 0c 00 00 8b 15 e3 55 5f 07 85 d2 75 29 48 8d 7d 60 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 db 0e 00 00 48 3b 6d 60 0f 85 5a 0a 00 00 bf 01
> > RSP: 0018:ffffc90000cd7b78 EFLAGS: 00010216
> > RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000001
> > RDX: 000000000000005a RSI: 0000000000000000 RDI: 00000000000002d0
> > RBP: 0000000000000270 R08: ffffffff880a40d9 R09: 0000000000000000
> > R10: ffffffff814b4be0 R11: 0000000000000000 R12: 0000000000000000
> > R13: dffffc0000000000 R14: ffff888072e47020 R15: ffff8880b9c34a40
> > FS: 0000000000000000(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007fcac15f1d58 CR3: 00000000628fa000 CR4: 0000000000350ef0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> > Call Trace:
> > l2cap_chan_timeout+0x69/0x2f0 net/bluetooth/l2cap_core.c:422
> > process_one_work+0x98d/0x1600 kernel/workqueue.c:2276
> > worker_thread+0x64c/0x1120 kernel/workqueue.c:2422
> > kthread+0x3b1/0x4a0 kernel/kthread.c:313
> > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
>
> To fix the uaf reported, 1) releases connection through rcu 2) detects race
> under rcu lock in the delayed work callback.
>
> Note it is only for idea and thoughts are welcome if it makes sense to you.
>
> +++ x/net/bluetooth/l2cap_core.c
> @@ -414,12 +414,25 @@ static void l2cap_chan_timeout(struct wo
> {
> struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
> chan_timer.work);
> - struct l2cap_conn *conn = chan->conn;
> + struct l2cap_conn *conn;
> int reason;
>
> + rcu_read_lock();
> + conn = chan->conn;
> + if (conn && !kref_get_unless_zero(&conn->ref))
> + conn = NULL;
> + rcu_read_unlock();
> +
> + if (!conn)
> + goto put;
> +
> BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
>
> mutex_lock(&conn->chan_lock);
> +
> + if (!chan->conn)
> + goto put;
> +
> /* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling
> * this work. No need to call l2cap_chan_hold(chan) here again.
> */
> @@ -438,9 +451,13 @@ static void l2cap_chan_timeout(struct wo
> chan->ops->close(chan);
>
> l2cap_chan_unlock(chan);
> +put:
> l2cap_chan_put(chan);
>
> + if (!conn)
> + return;
> mutex_unlock(&conn->chan_lock);
> + l2cap_conn_put(conn);
> }
>
> struct l2cap_chan *l2cap_chan_create(void)
> @@ -1915,12 +1932,19 @@ static void l2cap_conn_del(struct hci_co
> l2cap_conn_put(conn);
> }
>
> +static void l2cap_conn_rcu_fn(struct rcu_head *r)
> +{
> + struct l2cap_conn *conn = container_of(r, struct l2cap_conn, rcu);
> +
> + kfree(conn);
> +}
> +
> static void l2cap_conn_free(struct kref *ref)
> {
> struct l2cap_conn *conn = container_of(ref, struct l2cap_conn, ref);
>
> hci_conn_put(conn->hcon);
> - kfree(conn);
Shouldn't we actually cancel the timeout work if the connection is
freed here? At least I don't see a valid reason to have a l2cap_chan
without l2cap_conn.
> + call_rcu(&conn->rcu, l2cap_conn_rcu_fn);
> }
>
> struct l2cap_conn *l2cap_conn_get(struct l2cap_conn *conn)
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [syzbot] general protection fault in l2cap_chan_timeout (2)
[not found] ` <20210601075417.2763-1-hdanton@sina.com>
@ 2021-06-01 20:48 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-06-01 20:48 UTC (permalink / raw)
To: Hillf Danton
Cc: syzbot, David Miller, Johan Hedberg, Jakub Kicinski,
linux-bluetooth, Linux Kernel Mailing List, Marcel Holtmann,
open list:NETWORKING [GENERAL],
syzkaller-bugs
Hi,
On Tue, Jun 1, 2021 at 12:54 AM Hillf Danton <hdanton@sina.com> wrote:
>
> On Mon, 31 May 2021 19:11:08 -0700 Luiz Augusto von Dentz wrote:
> >
> >Shouldn't we actually cancel the timeout work if the connection is
> >freed here? At least I don't see a valid reason to have a l2cap_chan
> >without l2cap_conn.
>
> A far neater approach at the cost of making l2cap_conn_put() blocking and
> nobody currently seems to care about it.
I wonder what is going on here, there doesn't seem to be any code path
where the chan_timer is not cleared since the code path should be:
l2cap_conn_del -> l2cap_chan_del -> __clear_chan_timer -> cancel_delayed_work
chan->conn = NULL
Perhaps the problem is that cancel_delayed_work does not actually
prevent l2cap_chan_timeout to run if that is already pending, so maybe
something like this would work:
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9ebb85df4db4..f6e423111dfc 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -414,17 +414,23 @@ static void l2cap_chan_timeout(struct work_struct *work)
{
struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
chan_timer.work);
- struct l2cap_conn *conn = chan->conn;
+ struct l2cap_conn *conn;
int reason;
BT_DBG("chan %p state %s", chan, state_to_string(chan->state));
- mutex_lock(&conn->chan_lock);
/* __set_chan_timer() calls l2cap_chan_hold(chan) while scheduling
* this work. No need to call l2cap_chan_hold(chan) here again.
*/
l2cap_chan_lock(chan);
+ conn = chan->conn;
+ if (!conn)
+ /* l2cap_conn_del might have run */
+ goto unlock;
+
+ mutex_lock(&conn->chan_lock);
+
if (chan->state == BT_CONNECTED || chan->state == BT_CONFIG)
reason = ECONNREFUSED;
else if (chan->state == BT_CONNECT &&
@@ -437,10 +443,11 @@ static void l2cap_chan_timeout(struct work_struct *work)
chan->ops->close(chan);
+ mutex_unlock(&conn->chan_lock);
+
+unlock:
l2cap_chan_unlock(chan);
l2cap_chan_put(chan);
-
- mutex_unlock(&conn->chan_lock);
}
--
Luiz Augusto von Dentz
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-01 20:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31 7:19 [syzbot] general protection fault in l2cap_chan_timeout (2) syzbot
[not found] ` <20210531090414.2558-1-hdanton@sina.com>
2021-06-01 2:11 ` Luiz Augusto von Dentz
[not found] ` <20210601075417.2763-1-hdanton@sina.com>
2021-06-01 20:48 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).