Fix the race condition between the following two flows that run in
parallel:
1. l2cap_reassemble_sdu -> chan->ops->recv -> l2cap_sock_recv_cb ->
__sock_queue_rcv_skb.
2. bt_sock_recvmsg -> skb_recv_datagram, skb_free_datagram.
An SKB can be queued by the first flow and immediately dequeued and
freed by the second flow, therefore the callers of l2cap_reassemble_sdu
can't use the SKB after that function returns. However, some places
continue accessing struct l2cap_ctrl that resides in the SKB's CB for a
short time after l2cap_reassemble_sdu returns, leading to a
use-after-free condition (the stack trace is below, line numbers for
kernel 5.19.8).
Fix it by keeping a local copy of struct l2cap_ctrl.
BUG: KASAN: use-after-free in l2cap_rx_state_recv (net/bluetooth/l2cap_core.c:6906) bluetooth
Read of size 1 at addr ffff88812025f2f0 by task kworker/u17:3/43169
Workqueue: hci0 hci_rx_work [bluetooth]
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:107 (discriminator 4))
print_report.cold (mm/kasan/report.c:314 mm/kasan/report.c:429)
? l2cap_rx_state_recv (net/bluetooth/l2cap_core.c:6906) bluetooth
kasan_report (mm/kasan/report.c:162 mm/kasan/report.c:493)
? l2cap_rx_state_recv (net/bluetooth/l2cap_core.c:6906) bluetooth
l2cap_rx_state_recv (net/bluetooth/l2cap_core.c:6906) bluetooth
l2cap_rx (net/bluetooth/l2cap_core.c:7236 net/bluetooth/l2cap_core.c:7271) bluetooth
? sk_filter_trim_cap (net/core/filter.c:123)
? l2cap_chan_hold_unless_zero (./arch/x86/include/asm/atomic.h:202 ./include/linux/atomic/atomic-instrumented.h:560 ./include/linux/refcount.h:157 ./include/linux/refcount.h:227 ./include/linux/refcount.h:245 ./include/linux/kref.h:111 net/bluetooth/l2cap_core.c:517) bluetooth
? l2cap_rx_state_recv (net/bluetooth/l2cap_core.c:7252) bluetooth
l2cap_recv_frame (net/bluetooth/l2cap_core.c:7405 net/bluetooth/l2cap_core.c:7620 net/bluetooth/l2cap_core.c:7723) bluetooth
? lock_release (./include/trace/events/lock.h:69 kernel/locking/lockdep.c:5677)
? hci_rx_work (net/bluetooth/hci_core.c:3637 net/bluetooth/hci_core.c:3832) bluetooth
? reacquire_held_locks (kernel/locking/lockdep.c:5674)
? trace_contention_end (./include/trace/events/lock.h:122 ./include/trace/events/lock.h:122)
? l2cap_config_rsp.isra.0 (net/bluetooth/l2cap_core.c:7674) bluetooth
? hci_rx_work (./include/net/bluetooth/hci_core.h:1024 net/bluetooth/hci_core.c:3634 net/bluetooth/hci_core.c:3832) bluetooth
? lock_acquire (./include/trace/events/lock.h:24 kernel/locking/lockdep.c:5637)
? __mutex_unlock_slowpath (./arch/x86/include/asm/atomic64_64.h:190 ./include/linux/atomic/atomic-long.h:449 ./include/linux/atomic/atomic-instrumented.h:1790 kernel/locking/mutex.c:924)
? rcu_read_lock_sched_held (kernel/rcu/update.c:104 kernel/rcu/update.c:123)
? memcpy (mm/kasan/shadow.c:65 (discriminator 1))
? l2cap_recv_frag (net/bluetooth/l2cap_core.c:8340) bluetooth
l2cap_recv_acldata (net/bluetooth/l2cap_core.c:8486) bluetooth
hci_rx_work (net/bluetooth/hci_core.c:3642 net/bluetooth/hci_core.c:3832) bluetooth
process_one_work (kernel/workqueue.c:2289)
? lock_downgrade (kernel/locking/lockdep.c:5634)
? pwq_dec_nr_in_flight (kernel/workqueue.c:2184)
? rwlock_bug.part.0 (kernel/locking/spinlock_debug.c:113)
worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2437)
? __kthread_parkme (./arch/x86/include/asm/bitops.h:207 (discriminator 4) ./include/asm-generic/bitops/instrumented-non-atomic.h:135 (discriminator 4) kernel/kthread.c:270 (discriminator 4))
? process_one_work (kernel/workqueue.c:2379)
kthread (kernel/kthread.c:376)
? kthread_complete_and_exit (kernel/kthread.c:331)
ret_from_fork (arch/x86/entry/entry_64.S:306)
</TASK>
Allocated by task 43169:
kasan_save_stack (mm/kasan/common.c:39)
__kasan_slab_alloc (mm/kasan/common.c:45 mm/kasan/common.c:436 mm/kasan/common.c:469)
kmem_cache_alloc_node (mm/slab.h:750 mm/slub.c:3243 mm/slub.c:3293)
__alloc_skb (net/core/skbuff.c:414)
l2cap_recv_frag (./include/net/bluetooth/bluetooth.h:425 net/bluetooth/l2cap_core.c:8329) bluetooth
l2cap_recv_acldata (net/bluetooth/l2cap_core.c:8442) bluetooth
hci_rx_work (net/bluetooth/hci_core.c:3642 net/bluetooth/hci_core.c:3832) bluetooth
process_one_work (kernel/workqueue.c:2289)
worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2437)
kthread (kernel/kthread.c:376)
ret_from_fork (arch/x86/entry/entry_64.S:306)
Freed by task 27920:
kasan_save_stack (mm/kasan/common.c:39)
kasan_set_track (mm/kasan/common.c:45)
kasan_set_free_info (mm/kasan/generic.c:372)
____kasan_slab_free (mm/kasan/common.c:368 mm/kasan/common.c:328)
slab_free_freelist_hook (mm/slub.c:1780)
kmem_cache_free (mm/slub.c:3536 mm/slub.c:3553)
skb_free_datagram (./include/net/sock.h:1578 ./include/net/sock.h:1639 net/core/datagram.c:323)
bt_sock_recvmsg (net/bluetooth/af_bluetooth.c:295) bluetooth
l2cap_sock_recvmsg (net/bluetooth/l2cap_sock.c:1212) bluetooth
sock_read_iter (net/socket.c:1087)
new_sync_read (./include/linux/fs.h:2052 fs/read_write.c:401)
vfs_read (fs/read_write.c:482)
ksys_read (fs/read_write.c:620)
do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
Link: https://lore.kernel.org/linux-bluetooth/CAKErNvoqga1WcmoR3-0875esY6TVWFQDandbVZncSiuGPBQXLA@mail.gmail.com/T/#u
Fixes: d2a7ac5d5d3a ("Bluetooth: Add the ERTM receive state machine")
Fixes: 4b51dae96731 ("Bluetooth: Add streaming mode receive and incoming packet classifier")
Signed-off-by: Maxim Mikityanskiy <[email protected]>
---
net/bluetooth/l2cap_core.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 2c9de67daadc..ea0929458f45 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -6886,6 +6886,8 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
case L2CAP_EV_RECV_IFRAME:
switch (l2cap_classify_txseq(chan, control->txseq)) {
case L2CAP_TXSEQ_EXPECTED:
+ struct l2cap_ctrl local_control;
+
l2cap_pass_to_tx(chan, control);
if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
@@ -6900,15 +6902,16 @@ static int l2cap_rx_state_recv(struct l2cap_chan *chan,
chan->buffer_seq = chan->expected_tx_seq;
skb_in_use = true;
+ local_control = *control;
err = l2cap_reassemble_sdu(chan, skb, control);
if (err)
break;
- if (control->final) {
+ if (local_control.final) {
if (!test_and_clear_bit(CONN_REJ_ACT,
&chan->conn_state)) {
- control->final = 0;
- l2cap_retransmit_all(chan, control);
+ local_control.final = 0;
+ l2cap_retransmit_all(chan, &local_control);
l2cap_ertm_send(chan);
}
}
@@ -7288,11 +7291,12 @@ static int l2cap_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
static int l2cap_stream_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
struct sk_buff *skb)
{
+ u16 txseq = control->txseq;
+
BT_DBG("chan %p, control %p, skb %p, state %d", chan, control, skb,
chan->rx_state);
- if (l2cap_classify_txseq(chan, control->txseq) ==
- L2CAP_TXSEQ_EXPECTED) {
+ if (l2cap_classify_txseq(chan, txseq) == L2CAP_TXSEQ_EXPECTED) {
l2cap_pass_to_tx(chan, control);
BT_DBG("buffer_seq %u->%u", chan->buffer_seq,
@@ -7315,8 +7319,8 @@ static int l2cap_stream_rx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
}
}
- chan->last_acked_seq = control->txseq;
- chan->expected_tx_seq = __next_seq(chan, control->txseq);
+ chan->last_acked_seq = txseq;
+ chan->expected_tx_seq = __next_seq(chan, txseq);
return 0;
}
--
2.37.3
Hi Maxim,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on bluetooth/master]
[also build test ERROR on bluetooth-next/master mptcp/export linus/master v6.0 next-20220930]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Maxim-Mikityanskiy/Bluetooth-L2CAP-Fix-use-after-free-caused-by-l2cap_reassemble_sdu/20221003-061206
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
config: arm-randconfig-r023-20221003
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/72e1f19d6b44551bdc1bf570f9be071ad4e0284d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Maxim-Mikityanskiy/Bluetooth-L2CAP-Fix-use-after-free-caused-by-l2cap_reassemble_sdu/20221003-061206
git checkout 72e1f19d6b44551bdc1bf570f9be071ad4e0284d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash net/bluetooth/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> net/bluetooth/l2cap_core.c:6889:4: error: expected expression
struct l2cap_ctrl local_control;
^
>> net/bluetooth/l2cap_core.c:6905:4: error: use of undeclared identifier 'local_control'
local_control = *control;
^
net/bluetooth/l2cap_core.c:6910:8: error: use of undeclared identifier 'local_control'
if (local_control.final) {
^
net/bluetooth/l2cap_core.c:6913:6: error: use of undeclared identifier 'local_control'
local_control.final = 0;
^
net/bluetooth/l2cap_core.c:6914:34: error: use of undeclared identifier 'local_control'; did you mean '__pack_control'?
l2cap_retransmit_all(chan, &local_control);
^~~~~~~~~~~~~
__pack_control
net/bluetooth/l2cap_core.c:1115:20: note: '__pack_control' declared here
static inline void __pack_control(struct l2cap_chan *chan,
^
5 errors generated.
vim +6889 net/bluetooth/l2cap_core.c
6874
6875 static int l2cap_rx_state_recv(struct l2cap_chan *chan,
6876 struct l2cap_ctrl *control,
6877 struct sk_buff *skb, u8 event)
6878 {
6879 int err = 0;
6880 bool skb_in_use = false;
6881
6882 BT_DBG("chan %p, control %p, skb %p, event %d", chan, control, skb,
6883 event);
6884
6885 switch (event) {
6886 case L2CAP_EV_RECV_IFRAME:
6887 switch (l2cap_classify_txseq(chan, control->txseq)) {
6888 case L2CAP_TXSEQ_EXPECTED:
> 6889 struct l2cap_ctrl local_control;
6890
6891 l2cap_pass_to_tx(chan, control);
6892
6893 if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
6894 BT_DBG("Busy, discarding expected seq %d",
6895 control->txseq);
6896 break;
6897 }
6898
6899 chan->expected_tx_seq = __next_seq(chan,
6900 control->txseq);
6901
6902 chan->buffer_seq = chan->expected_tx_seq;
6903 skb_in_use = true;
6904
> 6905 local_control = *control;
6906 err = l2cap_reassemble_sdu(chan, skb, control);
6907 if (err)
6908 break;
6909
6910 if (local_control.final) {
6911 if (!test_and_clear_bit(CONN_REJ_ACT,
6912 &chan->conn_state)) {
6913 local_control.final = 0;
6914 l2cap_retransmit_all(chan, &local_control);
6915 l2cap_ertm_send(chan);
6916 }
6917 }
6918
6919 if (!test_bit(CONN_LOCAL_BUSY, &chan->conn_state))
6920 l2cap_send_ack(chan);
6921 break;
6922 case L2CAP_TXSEQ_UNEXPECTED:
6923 l2cap_pass_to_tx(chan, control);
6924
6925 /* Can't issue SREJ frames in the local busy state.
6926 * Drop this frame, it will be seen as missing
6927 * when local busy is exited.
6928 */
6929 if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
6930 BT_DBG("Busy, discarding unexpected seq %d",
6931 control->txseq);
6932 break;
6933 }
6934
6935 /* There was a gap in the sequence, so an SREJ
6936 * must be sent for each missing frame. The
6937 * current frame is stored for later use.
6938 */
6939 skb_queue_tail(&chan->srej_q, skb);
6940 skb_in_use = true;
6941 BT_DBG("Queued %p (queue len %d)", skb,
6942 skb_queue_len(&chan->srej_q));
6943
6944 clear_bit(CONN_SREJ_ACT, &chan->conn_state);
6945 l2cap_seq_list_clear(&chan->srej_list);
6946 l2cap_send_srej(chan, control->txseq);
6947
6948 chan->rx_state = L2CAP_RX_STATE_SREJ_SENT;
6949 break;
6950 case L2CAP_TXSEQ_DUPLICATE:
6951 l2cap_pass_to_tx(chan, control);
6952 break;
6953 case L2CAP_TXSEQ_INVALID_IGNORE:
6954 break;
6955 case L2CAP_TXSEQ_INVALID:
6956 default:
6957 l2cap_send_disconn_req(chan, ECONNRESET);
6958 break;
6959 }
6960 break;
6961 case L2CAP_EV_RECV_RR:
6962 l2cap_pass_to_tx(chan, control);
6963 if (control->final) {
6964 clear_bit(CONN_REMOTE_BUSY, &chan->conn_state);
6965
6966 if (!test_and_clear_bit(CONN_REJ_ACT, &chan->conn_state) &&
6967 !__chan_is_moving(chan)) {
6968 control->final = 0;
6969 l2cap_retransmit_all(chan, control);
6970 }
6971
6972 l2cap_ertm_send(chan);
6973 } else if (control->poll) {
6974 l2cap_send_i_or_rr_or_rnr(chan);
6975 } else {
6976 if (test_and_clear_bit(CONN_REMOTE_BUSY,
6977 &chan->conn_state) &&
6978 chan->unacked_frames)
6979 __set_retrans_timer(chan);
6980
6981 l2cap_ertm_send(chan);
6982 }
6983 break;
6984 case L2CAP_EV_RECV_RNR:
6985 set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
6986 l2cap_pass_to_tx(chan, control);
6987 if (control && control->poll) {
6988 set_bit(CONN_SEND_FBIT, &chan->conn_state);
6989 l2cap_send_rr_or_rnr(chan, 0);
6990 }
6991 __clear_retrans_timer(chan);
6992 l2cap_seq_list_clear(&chan->retrans_list);
6993 break;
6994 case L2CAP_EV_RECV_REJ:
6995 l2cap_handle_rej(chan, control);
6996 break;
6997 case L2CAP_EV_RECV_SREJ:
6998 l2cap_handle_srej(chan, control);
6999 break;
7000 default:
7001 break;
7002 }
7003
7004 if (skb && !skb_in_use) {
7005 BT_DBG("Freeing %p", skb);
7006 kfree_skb(skb);
7007 }
7008
7009 return err;
7010 }
7011
--
0-DAY CI Kernel Test Service
https://01.org/lkp