* [PATCH v5 0/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() @ 2026-05-20 16:20 Siwei Zhang 2026-05-20 16:20 ` [PATCH v5 1/1] " Siwei Zhang 0 siblings, 1 reply; 8+ messages in thread From: Siwei Zhang @ 2026-05-20 16:20 UTC (permalink / raw) To: Marcel Holtmann, Luiz Augusto von Dentz; +Cc: linux-bluetooth, Siwei Zhang Compared to v2, addresses comments on https://sashiko.dev/#/patchset/20260415204842.2363950-1-oss%40fourdim.xyz . Compared to v3, rebase against bluetooth-next. Compared to v4, allocate the channel outside the function and pass it in as an argument to avoid the use-after-free. Siwei Zhang (1): Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() include/net/bluetooth/l2cap.h | 8 +++-- net/bluetooth/6lowpan.c | 14 ++++----- net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ net/bluetooth/smp.c | 13 +++----- 5 files changed, 91 insertions(+), 50 deletions(-) -- 2.54.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-20 16:20 [PATCH v5 0/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() Siwei Zhang @ 2026-05-20 16:20 ` Siwei Zhang 2026-05-20 18:07 ` bluez.test.bot ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Siwei Zhang @ 2026-05-20 16:20 UTC (permalink / raw) To: Marcel Holtmann, Luiz Augusto von Dentz; +Cc: linux-bluetooth, Siwei Zhang l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after release_sock(parent). Once the parent lock is released, the child socket sk can be freed by another task. Allocate the channel outside the func to prevent this. Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c") Cc: stable@kernel.org Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Siwei Zhang <oss@fourdim.xyz> --- include/net/bluetooth/l2cap.h | 8 +++-- net/bluetooth/6lowpan.c | 14 ++++----- net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ net/bluetooth/smp.c | 13 +++----- 5 files changed, 91 insertions(+), 50 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 5172afee5494..f7a11e6431f0 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -619,7 +619,8 @@ struct l2cap_chan { struct l2cap_ops { char *name; - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan); + int (*new_connection)(struct l2cap_chan *chan, + struct l2cap_chan *new_chan); int (*recv) (struct l2cap_chan * chan, struct sk_buff *skb); void (*teardown) (struct l2cap_chan *chan, int err); @@ -883,9 +884,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) return (seq + 1) % (chan->tx_win_max + 1); } -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan) +static inline int l2cap_chan_no_new_connection(struct l2cap_chan *chan, + struct l2cap_chan *new_chan) { - return NULL; + return -EOPNOTSUPP; } static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb) diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c index 23a229ab6a33..286c0b45055b 100644 --- a/net/bluetooth/6lowpan.c +++ b/net/bluetooth/6lowpan.c @@ -743,19 +743,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan) ifup(dev->netdev); } -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan) +static inline int chan_new_conn_cb(struct l2cap_chan *pchan, + struct l2cap_chan *chan) { - struct l2cap_chan *chan; - - chan = chan_create(); - if (!chan) - return NULL; + l2cap_chan_set_defaults(chan); + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; + chan->mode = L2CAP_MODE_LE_FLOWCTL; + chan->imtu = 1280; chan->ops = pchan->ops; BT_DBG("chan %p pchan %p", chan, pchan); - return chan; + return 0; } static void unregister_dev(struct lowpan_btle_dev *dev) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index fdccd62ccca8..505f32034971 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -4051,10 +4051,16 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, goto response; } - chan = pchan->ops->new_connection(pchan); + chan = l2cap_chan_create(); if (!chan) goto response; + if (pchan->ops->new_connection(pchan, chan) < 0) { + l2cap_chan_put(chan); + chan = NULL; + goto response; + } + /* For certain devices (ex: HID mouse), support for authentication, * pairing and bonding is optional. For such devices, inorder to avoid * the ACL alive for too long after L2CAP disconnection, reset the ACL @@ -4132,6 +4138,10 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, chan->num_conf_req++; } + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ + if (chan) + l2cap_chan_put(chan); + l2cap_chan_unlock(pchan); l2cap_chan_put(pchan); } @@ -4881,6 +4891,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, struct l2cap_le_conn_rsp rsp; struct l2cap_chan *chan, *pchan; u16 dcid, scid, credits, mtu, mps; + u16 rsp_mtu, rsp_mps; __le16 psm; u8 result; @@ -4893,6 +4904,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, psm = req->psm; dcid = 0; credits = 0; + rsp_mtu = 0; + rsp_mps = 0; if (mtu < 23 || mps < 23) return -EPROTO; @@ -4953,12 +4966,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, goto response_unlock; } - chan = pchan->ops->new_connection(pchan); + chan = l2cap_chan_create(); if (!chan) { result = L2CAP_CR_LE_NO_MEM; goto response_unlock; } + if (pchan->ops->new_connection(pchan, chan) < 0) { + l2cap_chan_put(chan); + chan = NULL; + result = L2CAP_CR_LE_NO_MEM; + goto response_unlock; + } + bacpy(&chan->src, &conn->hcon->src); bacpy(&chan->dst, &conn->hcon->dst); chan->src_type = bdaddr_src_type(conn->hcon); @@ -4974,6 +4994,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, dcid = chan->scid; credits = chan->rx_credits; + rsp_mtu = chan->imtu; + rsp_mps = chan->mps; __set_chan_timer(chan, chan->ops->get_sndtimeo(chan)); @@ -4993,6 +5015,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, result = L2CAP_CR_LE_SUCCESS; } + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ + l2cap_chan_put(chan); + response_unlock: l2cap_chan_unlock(pchan); l2cap_chan_put(pchan); @@ -5001,13 +5026,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, return 0; response: - if (chan) { - rsp.mtu = cpu_to_le16(chan->imtu); - rsp.mps = cpu_to_le16(chan->mps); - } else { - rsp.mtu = 0; - rsp.mps = 0; - } + rsp.mtu = cpu_to_le16(rsp_mtu); + rsp.mps = cpu_to_le16(rsp_mps); rsp.dcid = cpu_to_le16(dcid); rsp.credits = cpu_to_le16(credits); @@ -5177,12 +5197,18 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, continue; } - chan = pchan->ops->new_connection(pchan); + chan = l2cap_chan_create(); if (!chan) { result = L2CAP_CR_LE_NO_MEM; continue; } + if (pchan->ops->new_connection(pchan, chan) < 0) { + l2cap_chan_put(chan); + result = L2CAP_CR_LE_NO_MEM; + continue; + } + bacpy(&chan->src, &conn->hcon->src); bacpy(&chan->dst, &conn->hcon->dst); chan->src_type = bdaddr_src_type(conn->hcon); @@ -5217,6 +5243,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, } else { l2cap_chan_ready(chan); } + + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ + l2cap_chan_put(chan); } unlock: @@ -7399,7 +7428,11 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) goto next; l2cap_chan_lock(pchan); - chan = pchan->ops->new_connection(pchan); + chan = l2cap_chan_create(); + if (chan && pchan->ops->new_connection(pchan, chan) < 0) { + l2cap_chan_put(chan); + chan = NULL; + } if (chan) { bacpy(&chan->src, &hcon->src); bacpy(&chan->dst, &hcon->dst); @@ -7407,6 +7440,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) chan->dst_type = dst_type; __l2cap_chan_add(conn, chan); + + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ + l2cap_chan_put(chan); } l2cap_chan_unlock(pchan); diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index dede550d6031..598f24c8f704 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = { static const struct proto_ops l2cap_sock_ops; static void l2cap_sock_init(struct sock *sk, struct sock *parent); static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, - int proto, gfp_t prio, int kern); + int proto, gfp_t prio, int kern, + struct l2cap_chan *chan); static void l2cap_sock_cleanup_listen(struct sock *parent); bool l2cap_is_socket(struct socket *sock) @@ -1507,12 +1508,13 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) } } -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) +static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan, + struct l2cap_chan *new_chan) { struct sock *sk, *parent = chan->data; if (!parent) - return NULL; + return -EINVAL; lock_sock(parent); @@ -1520,15 +1522,15 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) if (sk_acceptq_is_full(parent)) { BT_DBG("backlog full %d", parent->sk_ack_backlog); release_sock(parent); - return NULL; + return -ENOBUFS; } sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, - GFP_ATOMIC, 0); + GFP_ATOMIC, 0, new_chan); if (!sk) { release_sock(parent); - return NULL; - } + return -ENOMEM; + } bt_sock_reclassify_lock(sk, BTPROTO_L2CAP); @@ -1538,7 +1540,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) release_sock(parent); - return l2cap_pi(sk)->chan; + return 0; } static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) @@ -1939,10 +1941,10 @@ static struct proto l2cap_proto = { }; static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, - int proto, gfp_t prio, int kern) + int proto, gfp_t prio, int kern, + struct l2cap_chan *chan) { struct sock *sk; - struct l2cap_chan *chan; sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern); if (!sk) @@ -1953,14 +1955,11 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy); - chan = l2cap_chan_create(); - if (!chan) { - sk_free(sk); - if (sock) - sock->sk = NULL; - return NULL; - } - + /* The sock owns two refs on chan, matching the puts in + * l2cap_sock_kill() and l2cap_sock_destruct(). The caller keeps + * its own ref independent of these. + */ + l2cap_chan_hold(chan); l2cap_chan_hold(chan); l2cap_pi(sk)->chan = chan; @@ -1972,6 +1971,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, int kern) { struct sock *sk; + struct l2cap_chan *chan; BT_DBG("sock %p", sock); @@ -1986,10 +1986,18 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, sock->ops = &l2cap_sock_ops; - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern); - if (!sk) + chan = l2cap_chan_create(); + if (!chan) return -ENOMEM; + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan); + if (!sk) { + l2cap_chan_put(chan); + return -ENOMEM; + } + /* Sock has taken its own refs on chan; drop the chan_create() ref. */ + l2cap_chan_put(chan); + l2cap_sock_init(sk, NULL); bt_sock_link(&l2cap_sk_list, sk); return 0; diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 1739c1989dbd..25cb5dc580bf 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -3204,16 +3204,11 @@ static const struct l2cap_ops smp_chan_ops = { .get_sndtimeo = l2cap_chan_no_get_sndtimeo, }; -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) +static inline int smp_new_conn_cb(struct l2cap_chan *pchan, + struct l2cap_chan *chan) { - struct l2cap_chan *chan; - BT_DBG("pchan %p", pchan); - chan = l2cap_chan_create(); - if (!chan) - return NULL; - chan->chan_type = pchan->chan_type; chan->ops = &smp_chan_ops; chan->scid = pchan->scid; @@ -3229,9 +3224,9 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) */ atomic_set(&chan->nesting, L2CAP_NESTING_SMP); - BT_DBG("created chan %p", chan); + BT_DBG("initialised chan %p", chan); - return chan; + return 0; } static const struct l2cap_ops smp_root_chan_ops = { -- 2.54.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-20 16:20 ` [PATCH v5 1/1] " Siwei Zhang @ 2026-05-20 18:07 ` bluez.test.bot 2026-05-26 20:38 ` [PATCH v5 1/1] " Siwei Zhang 2026-05-26 20:46 ` Luiz Augusto von Dentz 2 siblings, 0 replies; 8+ messages in thread From: bluez.test.bot @ 2026-05-20 18:07 UTC (permalink / raw) To: linux-bluetooth, oss [-- Attachment #1: Type: text/plain, Size: 1045 bytes --] This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1098163 ---Test result--- Test Summary: CheckPatch PASS 1.67 seconds GitLint PASS 0.34 seconds SubjectPrefix PASS 0.13 seconds BuildKernel PASS 24.93 seconds CheckAllWarning PASS 27.79 seconds CheckSparse PASS 26.40 seconds BuildKernel32 PASS 24.49 seconds TestRunnerSetup PASS 521.38 seconds TestRunner_l2cap-tester PASS 376.22 seconds TestRunner_smp-tester PASS 17.96 seconds TestRunner_6lowpan-tester PASS 50.91 seconds IncrementalBuild PASS 24.22 seconds https://github.com/bluez/bluetooth-next/pull/221 --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-20 16:20 ` [PATCH v5 1/1] " Siwei Zhang 2026-05-20 18:07 ` bluez.test.bot @ 2026-05-26 20:38 ` Siwei Zhang 2026-05-26 20:46 ` Luiz Augusto von Dentz 2 siblings, 0 replies; 8+ messages in thread From: Siwei Zhang @ 2026-05-26 20:38 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, Marcel Holtmann Hi Luiz, On Wed, May 20, 2026, at 12:20 PM, Siwei Zhang wrote: > l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after > release_sock(parent). Once the parent lock is released, the child > socket sk can be freed by another task. > > Allocate the channel outside the func to prevent this. > > Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c") > Cc: stable@kernel.org > Assisted-by: Claude:claude-opus-4-6 > Signed-off-by: Siwei Zhang <oss@fourdim.xyz> > --- > include/net/bluetooth/l2cap.h | 8 +++-- > net/bluetooth/6lowpan.c | 14 ++++----- > net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- > net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ > net/bluetooth/smp.c | 13 +++----- > 5 files changed, 91 insertions(+), 50 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 5172afee5494..f7a11e6431f0 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -619,7 +619,8 @@ struct l2cap_chan { > struct l2cap_ops { > char *name; > > - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan); > + int (*new_connection)(struct l2cap_chan *chan, > + struct l2cap_chan *new_chan); > int (*recv) (struct l2cap_chan * chan, > struct sk_buff *skb); > void (*teardown) (struct l2cap_chan *chan, int err); > @@ -883,9 +884,10 @@ static inline __u16 __next_seq(struct l2cap_chan > *chan, __u16 seq) > return (seq + 1) % (chan->tx_win_max + 1); > } > > -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct > l2cap_chan *chan) > +static inline int l2cap_chan_no_new_connection(struct l2cap_chan *chan, > + struct l2cap_chan *new_chan) > { > - return NULL; > + return -EOPNOTSUPP; > } > > static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct > sk_buff *skb) > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c > index 23a229ab6a33..286c0b45055b 100644 > --- a/net/bluetooth/6lowpan.c > +++ b/net/bluetooth/6lowpan.c > @@ -743,19 +743,19 @@ static inline void chan_ready_cb(struct > l2cap_chan *chan) > ifup(dev->netdev); > } > > -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan) > +static inline int chan_new_conn_cb(struct l2cap_chan *pchan, > + struct l2cap_chan *chan) > { > - struct l2cap_chan *chan; > - > - chan = chan_create(); > - if (!chan) > - return NULL; > + l2cap_chan_set_defaults(chan); > > + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; > + chan->mode = L2CAP_MODE_LE_FLOWCTL; > + chan->imtu = 1280; > chan->ops = pchan->ops; > > BT_DBG("chan %p pchan %p", chan, pchan); > > - return chan; > + return 0; > } > > static void unregister_dev(struct lowpan_btle_dev *dev) > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index fdccd62ccca8..505f32034971 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -4051,10 +4051,16 @@ static void l2cap_connect(struct l2cap_conn > *conn, struct l2cap_cmd_hdr *cmd, > goto response; > } > > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > if (!chan) > goto response; > > + if (pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + chan = NULL; > + goto response; > + } > + > /* For certain devices (ex: HID mouse), support for authentication, > * pairing and bonding is optional. For such devices, inorder to avoid > * the ACL alive for too long after L2CAP disconnection, reset the ACL > @@ -4132,6 +4138,10 @@ static void l2cap_connect(struct l2cap_conn > *conn, struct l2cap_cmd_hdr *cmd, > chan->num_conf_req++; > } > > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + if (chan) > + l2cap_chan_put(chan); > + > l2cap_chan_unlock(pchan); > l2cap_chan_put(pchan); > } > @@ -4881,6 +4891,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > struct l2cap_le_conn_rsp rsp; > struct l2cap_chan *chan, *pchan; > u16 dcid, scid, credits, mtu, mps; > + u16 rsp_mtu, rsp_mps; > __le16 psm; > u8 result; > > @@ -4893,6 +4904,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > psm = req->psm; > dcid = 0; > credits = 0; > + rsp_mtu = 0; > + rsp_mps = 0; > > if (mtu < 23 || mps < 23) > return -EPROTO; > @@ -4953,12 +4966,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > goto response_unlock; > } > > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > if (!chan) { > result = L2CAP_CR_LE_NO_MEM; > goto response_unlock; > } > > + if (pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + chan = NULL; > + result = L2CAP_CR_LE_NO_MEM; > + goto response_unlock; > + } > + > bacpy(&chan->src, &conn->hcon->src); > bacpy(&chan->dst, &conn->hcon->dst); > chan->src_type = bdaddr_src_type(conn->hcon); > @@ -4974,6 +4994,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > > dcid = chan->scid; > credits = chan->rx_credits; > + rsp_mtu = chan->imtu; > + rsp_mps = chan->mps; > > __set_chan_timer(chan, chan->ops->get_sndtimeo(chan)); > > @@ -4993,6 +5015,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > result = L2CAP_CR_LE_SUCCESS; > } > > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + l2cap_chan_put(chan); > + > response_unlock: > l2cap_chan_unlock(pchan); > l2cap_chan_put(pchan); > @@ -5001,13 +5026,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > return 0; > > response: > - if (chan) { > - rsp.mtu = cpu_to_le16(chan->imtu); > - rsp.mps = cpu_to_le16(chan->mps); > - } else { > - rsp.mtu = 0; > - rsp.mps = 0; > - } > + rsp.mtu = cpu_to_le16(rsp_mtu); > + rsp.mps = cpu_to_le16(rsp_mps); > > rsp.dcid = cpu_to_le16(dcid); > rsp.credits = cpu_to_le16(credits); > @@ -5177,12 +5197,18 @@ static inline int l2cap_ecred_conn_req(struct > l2cap_conn *conn, > continue; > } > > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > if (!chan) { > result = L2CAP_CR_LE_NO_MEM; > continue; > } > > + if (pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + result = L2CAP_CR_LE_NO_MEM; > + continue; > + } > + > bacpy(&chan->src, &conn->hcon->src); > bacpy(&chan->dst, &conn->hcon->dst); > chan->src_type = bdaddr_src_type(conn->hcon); > @@ -5217,6 +5243,9 @@ static inline int l2cap_ecred_conn_req(struct > l2cap_conn *conn, > } else { > l2cap_chan_ready(chan); > } > + > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn > list. */ > + l2cap_chan_put(chan); > } > > unlock: > @@ -7399,7 +7428,11 @@ static void l2cap_connect_cfm(struct hci_conn > *hcon, u8 status) > goto next; > > l2cap_chan_lock(pchan); > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > + if (chan && pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + chan = NULL; > + } > if (chan) { > bacpy(&chan->src, &hcon->src); > bacpy(&chan->dst, &hcon->dst); > @@ -7407,6 +7440,9 @@ static void l2cap_connect_cfm(struct hci_conn > *hcon, u8 status) > chan->dst_type = dst_type; > > __l2cap_chan_add(conn, chan); > + > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + l2cap_chan_put(chan); > } > > l2cap_chan_unlock(pchan); > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c > index dede550d6031..598f24c8f704 100644 > --- a/net/bluetooth/l2cap_sock.c > +++ b/net/bluetooth/l2cap_sock.c > @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = { > static const struct proto_ops l2cap_sock_ops; > static void l2cap_sock_init(struct sock *sk, struct sock *parent); > static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > - int proto, gfp_t prio, int kern); > + int proto, gfp_t prio, int kern, > + struct l2cap_chan *chan); > static void l2cap_sock_cleanup_listen(struct sock *parent); > > bool l2cap_is_socket(struct socket *sock) > @@ -1507,12 +1508,13 @@ static void l2cap_sock_cleanup_listen(struct > sock *parent) > } > } > > -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > +static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan, > + struct l2cap_chan *new_chan) > { > struct sock *sk, *parent = chan->data; > > if (!parent) > - return NULL; > + return -EINVAL; > > lock_sock(parent); > > @@ -1520,15 +1522,15 @@ static struct l2cap_chan > *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > if (sk_acceptq_is_full(parent)) { > BT_DBG("backlog full %d", parent->sk_ack_backlog); > release_sock(parent); > - return NULL; > + return -ENOBUFS; > } > > sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, > - GFP_ATOMIC, 0); > + GFP_ATOMIC, 0, new_chan); > if (!sk) { > release_sock(parent); > - return NULL; > - } > + return -ENOMEM; > + } > > bt_sock_reclassify_lock(sk, BTPROTO_L2CAP); > > @@ -1538,7 +1540,7 @@ static struct l2cap_chan > *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > > release_sock(parent); > > - return l2cap_pi(sk)->chan; > + return 0; > } > > static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) > @@ -1939,10 +1941,10 @@ static struct proto l2cap_proto = { > }; > > static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > - int proto, gfp_t prio, int kern) > + int proto, gfp_t prio, int kern, > + struct l2cap_chan *chan) > { > struct sock *sk; > - struct l2cap_chan *chan; > > sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern); > if (!sk) > @@ -1953,14 +1955,11 @@ static struct sock *l2cap_sock_alloc(struct net > *net, struct socket *sock, > > INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy); > > - chan = l2cap_chan_create(); > - if (!chan) { > - sk_free(sk); > - if (sock) > - sock->sk = NULL; > - return NULL; > - } > - > + /* The sock owns two refs on chan, matching the puts in > + * l2cap_sock_kill() and l2cap_sock_destruct(). The caller keeps > + * its own ref independent of these. > + */ > + l2cap_chan_hold(chan); > l2cap_chan_hold(chan); > > l2cap_pi(sk)->chan = chan; > @@ -1972,6 +1971,7 @@ static int l2cap_sock_create(struct net *net, > struct socket *sock, int protocol, > int kern) > { > struct sock *sk; > + struct l2cap_chan *chan; > > BT_DBG("sock %p", sock); > > @@ -1986,10 +1986,18 @@ static int l2cap_sock_create(struct net *net, > struct socket *sock, int protocol, > > sock->ops = &l2cap_sock_ops; > > - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern); > - if (!sk) > + chan = l2cap_chan_create(); > + if (!chan) > return -ENOMEM; > > + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan); > + if (!sk) { > + l2cap_chan_put(chan); > + return -ENOMEM; > + } > + /* Sock has taken its own refs on chan; drop the chan_create() ref. */ > + l2cap_chan_put(chan); > + > l2cap_sock_init(sk, NULL); > bt_sock_link(&l2cap_sk_list, sk); > return 0; > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c > index 1739c1989dbd..25cb5dc580bf 100644 > --- a/net/bluetooth/smp.c > +++ b/net/bluetooth/smp.c > @@ -3204,16 +3204,11 @@ static const struct l2cap_ops smp_chan_ops = { > .get_sndtimeo = l2cap_chan_no_get_sndtimeo, > }; > > -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) > +static inline int smp_new_conn_cb(struct l2cap_chan *pchan, > + struct l2cap_chan *chan) > { > - struct l2cap_chan *chan; > - > BT_DBG("pchan %p", pchan); > > - chan = l2cap_chan_create(); > - if (!chan) > - return NULL; > - > chan->chan_type = pchan->chan_type; > chan->ops = &smp_chan_ops; > chan->scid = pchan->scid; > @@ -3229,9 +3224,9 @@ static inline struct l2cap_chan > *smp_new_conn_cb(struct l2cap_chan *pchan) > */ > atomic_set(&chan->nesting, L2CAP_NESTING_SMP); > > - BT_DBG("created chan %p", chan); > + BT_DBG("initialised chan %p", chan); > > - return chan; > + return 0; > } > > static const struct l2cap_ops smp_root_chan_ops = { > -- > 2.54.0 Could you please check this? Best, Siwei ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-20 16:20 ` [PATCH v5 1/1] " Siwei Zhang 2026-05-20 18:07 ` bluez.test.bot 2026-05-26 20:38 ` [PATCH v5 1/1] " Siwei Zhang @ 2026-05-26 20:46 ` Luiz Augusto von Dentz 2026-05-27 4:16 ` Siwei Zhang 2 siblings, 1 reply; 8+ messages in thread From: Luiz Augusto von Dentz @ 2026-05-26 20:46 UTC (permalink / raw) To: Siwei Zhang; +Cc: Marcel Holtmann, linux-bluetooth Hi Siwei, On Wed, May 20, 2026 at 12:20 PM Siwei Zhang <oss@fourdim.xyz> wrote: > > l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after > release_sock(parent). Once the parent lock is released, the child > socket sk can be freed by another task. > > Allocate the channel outside the func to prevent this. > > Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c") > Cc: stable@kernel.org > Assisted-by: Claude:claude-opus-4-6 > Signed-off-by: Siwei Zhang <oss@fourdim.xyz> > --- > include/net/bluetooth/l2cap.h | 8 +++-- > net/bluetooth/6lowpan.c | 14 ++++----- > net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- > net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ > net/bluetooth/smp.c | 13 +++----- > 5 files changed, 91 insertions(+), 50 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 5172afee5494..f7a11e6431f0 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -619,7 +619,8 @@ struct l2cap_chan { > struct l2cap_ops { > char *name; > > - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan); > + int (*new_connection)(struct l2cap_chan *chan, > + struct l2cap_chan *new_chan); > int (*recv) (struct l2cap_chan * chan, > struct sk_buff *skb); > void (*teardown) (struct l2cap_chan *chan, int err); > @@ -883,9 +884,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) > return (seq + 1) % (chan->tx_win_max + 1); > } > > -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan) > +static inline int l2cap_chan_no_new_connection(struct l2cap_chan *chan, > + struct l2cap_chan *new_chan) > { > - return NULL; > + return -EOPNOTSUPP; > } > > static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb) > diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c > index 23a229ab6a33..286c0b45055b 100644 > --- a/net/bluetooth/6lowpan.c > +++ b/net/bluetooth/6lowpan.c > @@ -743,19 +743,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan) > ifup(dev->netdev); > } > > -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan) > +static inline int chan_new_conn_cb(struct l2cap_chan *pchan, > + struct l2cap_chan *chan) > { > - struct l2cap_chan *chan; > - > - chan = chan_create(); > - if (!chan) > - return NULL; > + l2cap_chan_set_defaults(chan); > > + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; > + chan->mode = L2CAP_MODE_LE_FLOWCTL; > + chan->imtu = 1280; The 3 lines above make no sense. > chan->ops = pchan->ops; > > BT_DBG("chan %p pchan %p", chan, pchan); > > - return chan; > + return 0; > } > > static void unregister_dev(struct lowpan_btle_dev *dev) > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index fdccd62ccca8..505f32034971 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -4051,10 +4051,16 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, > goto response; > } > > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > if (!chan) > goto response; > > + if (pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + chan = NULL; > + goto response; > + } > + > /* For certain devices (ex: HID mouse), support for authentication, > * pairing and bonding is optional. For such devices, inorder to avoid > * the ACL alive for too long after L2CAP disconnection, reset the ACL > @@ -4132,6 +4138,10 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, > chan->num_conf_req++; > } > > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + if (chan) > + l2cap_chan_put(chan); > + > l2cap_chan_unlock(pchan); > l2cap_chan_put(pchan); > } > @@ -4881,6 +4891,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > struct l2cap_le_conn_rsp rsp; > struct l2cap_chan *chan, *pchan; > u16 dcid, scid, credits, mtu, mps; > + u16 rsp_mtu, rsp_mps; > __le16 psm; > u8 result; > > @@ -4893,6 +4904,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > psm = req->psm; > dcid = 0; > credits = 0; > + rsp_mtu = 0; > + rsp_mps = 0; > > if (mtu < 23 || mps < 23) > return -EPROTO; > @@ -4953,12 +4966,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > goto response_unlock; > } > > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > if (!chan) { > result = L2CAP_CR_LE_NO_MEM; > goto response_unlock; > } > > + if (pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + chan = NULL; > + result = L2CAP_CR_LE_NO_MEM; > + goto response_unlock; > + } > + > bacpy(&chan->src, &conn->hcon->src); > bacpy(&chan->dst, &conn->hcon->dst); > chan->src_type = bdaddr_src_type(conn->hcon); > @@ -4974,6 +4994,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > > dcid = chan->scid; > credits = chan->rx_credits; > + rsp_mtu = chan->imtu; > + rsp_mps = chan->mps; > > __set_chan_timer(chan, chan->ops->get_sndtimeo(chan)); > > @@ -4993,6 +5015,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > result = L2CAP_CR_LE_SUCCESS; > } > > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + l2cap_chan_put(chan); > + > response_unlock: > l2cap_chan_unlock(pchan); > l2cap_chan_put(pchan); > @@ -5001,13 +5026,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > return 0; > > response: > - if (chan) { > - rsp.mtu = cpu_to_le16(chan->imtu); > - rsp.mps = cpu_to_le16(chan->mps); > - } else { > - rsp.mtu = 0; > - rsp.mps = 0; > - } > + rsp.mtu = cpu_to_le16(rsp_mtu); > + rsp.mps = cpu_to_le16(rsp_mps); > > rsp.dcid = cpu_to_le16(dcid); > rsp.credits = cpu_to_le16(credits); > @@ -5177,12 +5197,18 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, > continue; > } > > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > if (!chan) { > result = L2CAP_CR_LE_NO_MEM; > continue; > } > > + if (pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + result = L2CAP_CR_LE_NO_MEM; > + continue; > + } > + > bacpy(&chan->src, &conn->hcon->src); > bacpy(&chan->dst, &conn->hcon->dst); > chan->src_type = bdaddr_src_type(conn->hcon); > @@ -5217,6 +5243,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, > } else { > l2cap_chan_ready(chan); > } > + > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + l2cap_chan_put(chan); > } > > unlock: > @@ -7399,7 +7428,11 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > goto next; > > l2cap_chan_lock(pchan); > - chan = pchan->ops->new_connection(pchan); > + chan = l2cap_chan_create(); > + if (chan && pchan->ops->new_connection(pchan, chan) < 0) { > + l2cap_chan_put(chan); > + chan = NULL; > + } > if (chan) { > bacpy(&chan->src, &hcon->src); > bacpy(&chan->dst, &hcon->dst); > @@ -7407,6 +7440,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > chan->dst_type = dst_type; > > __l2cap_chan_add(conn, chan); > + > + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > + l2cap_chan_put(chan); > } > > l2cap_chan_unlock(pchan); > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c > index dede550d6031..598f24c8f704 100644 > --- a/net/bluetooth/l2cap_sock.c > +++ b/net/bluetooth/l2cap_sock.c > @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = { > static const struct proto_ops l2cap_sock_ops; > static void l2cap_sock_init(struct sock *sk, struct sock *parent); > static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > - int proto, gfp_t prio, int kern); > + int proto, gfp_t prio, int kern, > + struct l2cap_chan *chan); > static void l2cap_sock_cleanup_listen(struct sock *parent); > > bool l2cap_is_socket(struct socket *sock) > @@ -1507,12 +1508,13 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) > } > } > > -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > +static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan, > + struct l2cap_chan *new_chan) > { > struct sock *sk, *parent = chan->data; > > if (!parent) > - return NULL; > + return -EINVAL; > > lock_sock(parent); > > @@ -1520,15 +1522,15 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > if (sk_acceptq_is_full(parent)) { > BT_DBG("backlog full %d", parent->sk_ack_backlog); > release_sock(parent); > - return NULL; > + return -ENOBUFS; > } > > sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, > - GFP_ATOMIC, 0); > + GFP_ATOMIC, 0, new_chan); > if (!sk) { > release_sock(parent); > - return NULL; > - } > + return -ENOMEM; > + } > > bt_sock_reclassify_lock(sk, BTPROTO_L2CAP); > > @@ -1538,7 +1540,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > > release_sock(parent); > > - return l2cap_pi(sk)->chan; > + return 0; > } > > static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) > @@ -1939,10 +1941,10 @@ static struct proto l2cap_proto = { > }; > > static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > - int proto, gfp_t prio, int kern) > + int proto, gfp_t prio, int kern, > + struct l2cap_chan *chan) > { > struct sock *sk; > - struct l2cap_chan *chan; > > sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern); > if (!sk) > @@ -1953,14 +1955,11 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > > INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy); > > - chan = l2cap_chan_create(); > - if (!chan) { > - sk_free(sk); > - if (sock) > - sock->sk = NULL; > - return NULL; > - } > - > + /* The sock owns two refs on chan, matching the puts in > + * l2cap_sock_kill() and l2cap_sock_destruct(). The caller keeps > + * its own ref independent of these. > + */ > + l2cap_chan_hold(chan); > l2cap_chan_hold(chan); > > l2cap_pi(sk)->chan = chan; > @@ -1972,6 +1971,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, > int kern) > { > struct sock *sk; > + struct l2cap_chan *chan; > > BT_DBG("sock %p", sock); > > @@ -1986,10 +1986,18 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, > > sock->ops = &l2cap_sock_ops; > > - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern); > - if (!sk) > + chan = l2cap_chan_create(); > + if (!chan) > return -ENOMEM; > > + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan); > + if (!sk) { > + l2cap_chan_put(chan); > + return -ENOMEM; > + } > + /* Sock has taken its own refs on chan; drop the chan_create() ref. */ > + l2cap_chan_put(chan); > + > l2cap_sock_init(sk, NULL); > bt_sock_link(&l2cap_sk_list, sk); > return 0; > diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c > index 1739c1989dbd..25cb5dc580bf 100644 > --- a/net/bluetooth/smp.c > +++ b/net/bluetooth/smp.c > @@ -3204,16 +3204,11 @@ static const struct l2cap_ops smp_chan_ops = { > .get_sndtimeo = l2cap_chan_no_get_sndtimeo, > }; > > -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) > +static inline int smp_new_conn_cb(struct l2cap_chan *pchan, > + struct l2cap_chan *chan) > { > - struct l2cap_chan *chan; > - > BT_DBG("pchan %p", pchan); > > - chan = l2cap_chan_create(); > - if (!chan) > - return NULL; > - > chan->chan_type = pchan->chan_type; > chan->ops = &smp_chan_ops; > chan->scid = pchan->scid; > @@ -3229,9 +3224,9 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) > */ > atomic_set(&chan->nesting, L2CAP_NESTING_SMP); > > - BT_DBG("created chan %p", chan); > + BT_DBG("initialised chan %p", chan); > > - return chan; > + return 0; > } > > static const struct l2cap_ops smp_root_chan_ops = { > -- > 2.54.0 > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-26 20:46 ` Luiz Augusto von Dentz @ 2026-05-27 4:16 ` Siwei Zhang 2026-05-29 16:33 ` Siwei Zhang 0 siblings, 1 reply; 8+ messages in thread From: Siwei Zhang @ 2026-05-27 4:16 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: Marcel Holtmann, linux-bluetooth Hi Luiz, On Tue, May 26, 2026, at 4:46 PM, Luiz Augusto von Dentz wrote: > Hi Siwei, > > On Wed, May 20, 2026 at 12:20 PM Siwei Zhang <oss@fourdim.xyz> wrote: >> >> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after >> release_sock(parent). Once the parent lock is released, the child >> socket sk can be freed by another task. >> >> Allocate the channel outside the func to prevent this. >> >> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c") >> Cc: stable@kernel.org >> Assisted-by: Claude:claude-opus-4-6 >> Signed-off-by: Siwei Zhang <oss@fourdim.xyz> >> --- >> include/net/bluetooth/l2cap.h | 8 +++-- >> net/bluetooth/6lowpan.c | 14 ++++----- >> net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- >> net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ >> net/bluetooth/smp.c | 13 +++----- >> 5 files changed, 91 insertions(+), 50 deletions(-) >> >> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h >> index 5172afee5494..f7a11e6431f0 100644 >> --- a/include/net/bluetooth/l2cap.h >> +++ b/include/net/bluetooth/l2cap.h >> @@ -619,7 +619,8 @@ struct l2cap_chan { >> struct l2cap_ops { >> char *name; >> >> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan); >> + int (*new_connection)(struct l2cap_chan *chan, >> + struct l2cap_chan *new_chan); >> int (*recv) (struct l2cap_chan * chan, >> struct sk_buff *skb); >> void (*teardown) (struct l2cap_chan *chan, int err); >> @@ -883,9 +884,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) >> return (seq + 1) % (chan->tx_win_max + 1); >> } >> >> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan) >> +static inline int l2cap_chan_no_new_connection(struct l2cap_chan *chan, >> + struct l2cap_chan *new_chan) >> { >> - return NULL; >> + return -EOPNOTSUPP; >> } >> >> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb) >> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c >> index 23a229ab6a33..286c0b45055b 100644 >> --- a/net/bluetooth/6lowpan.c >> +++ b/net/bluetooth/6lowpan.c >> @@ -743,19 +743,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan) >> ifup(dev->netdev); >> } >> >> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan) >> +static inline int chan_new_conn_cb(struct l2cap_chan *pchan, >> + struct l2cap_chan *chan) >> { >> - struct l2cap_chan *chan; >> - >> - chan = chan_create(); >> - if (!chan) >> - return NULL; >> + l2cap_chan_set_defaults(chan); >> >> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; >> + chan->mode = L2CAP_MODE_LE_FLOWCTL; >> + chan->imtu = 1280; > > The 3 lines above make no sense. > chan_create code in 6lowpan.c static struct l2cap_chan *chan_create(void) { struct l2cap_chan *chan; chan = l2cap_chan_create(); if (!chan) return NULL; l2cap_chan_set_defaults(chan); chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; chan->mode = L2CAP_MODE_LE_FLOWCTL; chan->imtu = 1280; return chan; } Since we allocate chan outside and replace the chan_create here, I do think these are needed and they are specific to 6lowpan only. I can refactor it in this patch or in a follow-up patch. I would prefer it to be in a follow-up patch. >> chan->ops = pchan->ops; >> >> BT_DBG("chan %p pchan %p", chan, pchan); >> >> - return chan; >> + return 0; >> } >> >> static void unregister_dev(struct lowpan_btle_dev *dev) >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >> index fdccd62ccca8..505f32034971 100644 >> --- a/net/bluetooth/l2cap_core.c >> +++ b/net/bluetooth/l2cap_core.c >> @@ -4051,10 +4051,16 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, >> goto response; >> } >> >> - chan = pchan->ops->new_connection(pchan); >> + chan = l2cap_chan_create(); >> if (!chan) >> goto response; >> >> + if (pchan->ops->new_connection(pchan, chan) < 0) { >> + l2cap_chan_put(chan); >> + chan = NULL; >> + goto response; >> + } >> + >> /* For certain devices (ex: HID mouse), support for authentication, >> * pairing and bonding is optional. For such devices, inorder to avoid >> * the ACL alive for too long after L2CAP disconnection, reset the ACL >> @@ -4132,6 +4138,10 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, >> chan->num_conf_req++; >> } >> >> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >> + if (chan) >> + l2cap_chan_put(chan); >> + >> l2cap_chan_unlock(pchan); >> l2cap_chan_put(pchan); >> } >> @@ -4881,6 +4891,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >> struct l2cap_le_conn_rsp rsp; >> struct l2cap_chan *chan, *pchan; >> u16 dcid, scid, credits, mtu, mps; >> + u16 rsp_mtu, rsp_mps; >> __le16 psm; >> u8 result; >> >> @@ -4893,6 +4904,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >> psm = req->psm; >> dcid = 0; >> credits = 0; >> + rsp_mtu = 0; >> + rsp_mps = 0; >> >> if (mtu < 23 || mps < 23) >> return -EPROTO; >> @@ -4953,12 +4966,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >> goto response_unlock; >> } >> >> - chan = pchan->ops->new_connection(pchan); >> + chan = l2cap_chan_create(); >> if (!chan) { >> result = L2CAP_CR_LE_NO_MEM; >> goto response_unlock; >> } >> >> + if (pchan->ops->new_connection(pchan, chan) < 0) { >> + l2cap_chan_put(chan); >> + chan = NULL; >> + result = L2CAP_CR_LE_NO_MEM; >> + goto response_unlock; >> + } >> + >> bacpy(&chan->src, &conn->hcon->src); >> bacpy(&chan->dst, &conn->hcon->dst); >> chan->src_type = bdaddr_src_type(conn->hcon); >> @@ -4974,6 +4994,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >> >> dcid = chan->scid; >> credits = chan->rx_credits; >> + rsp_mtu = chan->imtu; >> + rsp_mps = chan->mps; >> >> __set_chan_timer(chan, chan->ops->get_sndtimeo(chan)); >> >> @@ -4993,6 +5015,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >> result = L2CAP_CR_LE_SUCCESS; >> } >> >> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >> + l2cap_chan_put(chan); >> + >> response_unlock: >> l2cap_chan_unlock(pchan); >> l2cap_chan_put(pchan); >> @@ -5001,13 +5026,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >> return 0; >> >> response: >> - if (chan) { >> - rsp.mtu = cpu_to_le16(chan->imtu); >> - rsp.mps = cpu_to_le16(chan->mps); >> - } else { >> - rsp.mtu = 0; >> - rsp.mps = 0; >> - } >> + rsp.mtu = cpu_to_le16(rsp_mtu); >> + rsp.mps = cpu_to_le16(rsp_mps); >> >> rsp.dcid = cpu_to_le16(dcid); >> rsp.credits = cpu_to_le16(credits); >> @@ -5177,12 +5197,18 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, >> continue; >> } >> >> - chan = pchan->ops->new_connection(pchan); >> + chan = l2cap_chan_create(); >> if (!chan) { >> result = L2CAP_CR_LE_NO_MEM; >> continue; >> } >> >> + if (pchan->ops->new_connection(pchan, chan) < 0) { >> + l2cap_chan_put(chan); >> + result = L2CAP_CR_LE_NO_MEM; >> + continue; >> + } >> + >> bacpy(&chan->src, &conn->hcon->src); >> bacpy(&chan->dst, &conn->hcon->dst); >> chan->src_type = bdaddr_src_type(conn->hcon); >> @@ -5217,6 +5243,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, >> } else { >> l2cap_chan_ready(chan); >> } >> + >> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >> + l2cap_chan_put(chan); >> } >> >> unlock: >> @@ -7399,7 +7428,11 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) >> goto next; >> >> l2cap_chan_lock(pchan); >> - chan = pchan->ops->new_connection(pchan); >> + chan = l2cap_chan_create(); >> + if (chan && pchan->ops->new_connection(pchan, chan) < 0) { >> + l2cap_chan_put(chan); >> + chan = NULL; >> + } >> if (chan) { >> bacpy(&chan->src, &hcon->src); >> bacpy(&chan->dst, &hcon->dst); >> @@ -7407,6 +7440,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) >> chan->dst_type = dst_type; >> >> __l2cap_chan_add(conn, chan); >> + >> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >> + l2cap_chan_put(chan); >> } >> >> l2cap_chan_unlock(pchan); >> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c >> index dede550d6031..598f24c8f704 100644 >> --- a/net/bluetooth/l2cap_sock.c >> +++ b/net/bluetooth/l2cap_sock.c >> @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = { >> static const struct proto_ops l2cap_sock_ops; >> static void l2cap_sock_init(struct sock *sk, struct sock *parent); >> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, >> - int proto, gfp_t prio, int kern); >> + int proto, gfp_t prio, int kern, >> + struct l2cap_chan *chan); >> static void l2cap_sock_cleanup_listen(struct sock *parent); >> >> bool l2cap_is_socket(struct socket *sock) >> @@ -1507,12 +1508,13 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) >> } >> } >> >> -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) >> +static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan, >> + struct l2cap_chan *new_chan) >> { >> struct sock *sk, *parent = chan->data; >> >> if (!parent) >> - return NULL; >> + return -EINVAL; >> >> lock_sock(parent); >> >> @@ -1520,15 +1522,15 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) >> if (sk_acceptq_is_full(parent)) { >> BT_DBG("backlog full %d", parent->sk_ack_backlog); >> release_sock(parent); >> - return NULL; >> + return -ENOBUFS; >> } >> >> sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, >> - GFP_ATOMIC, 0); >> + GFP_ATOMIC, 0, new_chan); >> if (!sk) { >> release_sock(parent); >> - return NULL; >> - } >> + return -ENOMEM; >> + } >> >> bt_sock_reclassify_lock(sk, BTPROTO_L2CAP); >> >> @@ -1538,7 +1540,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) >> >> release_sock(parent); >> >> - return l2cap_pi(sk)->chan; >> + return 0; >> } >> >> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) >> @@ -1939,10 +1941,10 @@ static struct proto l2cap_proto = { >> }; >> >> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, >> - int proto, gfp_t prio, int kern) >> + int proto, gfp_t prio, int kern, >> + struct l2cap_chan *chan) >> { >> struct sock *sk; >> - struct l2cap_chan *chan; >> >> sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern); >> if (!sk) >> @@ -1953,14 +1955,11 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, >> >> INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy); >> >> - chan = l2cap_chan_create(); >> - if (!chan) { >> - sk_free(sk); >> - if (sock) >> - sock->sk = NULL; >> - return NULL; >> - } >> - >> + /* The sock owns two refs on chan, matching the puts in >> + * l2cap_sock_kill() and l2cap_sock_destruct(). The caller keeps >> + * its own ref independent of these. >> + */ >> + l2cap_chan_hold(chan); >> l2cap_chan_hold(chan); >> >> l2cap_pi(sk)->chan = chan; >> @@ -1972,6 +1971,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, >> int kern) >> { >> struct sock *sk; >> + struct l2cap_chan *chan; >> >> BT_DBG("sock %p", sock); >> >> @@ -1986,10 +1986,18 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, >> >> sock->ops = &l2cap_sock_ops; >> >> - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern); >> - if (!sk) >> + chan = l2cap_chan_create(); >> + if (!chan) >> return -ENOMEM; >> >> + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan); >> + if (!sk) { >> + l2cap_chan_put(chan); >> + return -ENOMEM; >> + } >> + /* Sock has taken its own refs on chan; drop the chan_create() ref. */ >> + l2cap_chan_put(chan); >> + >> l2cap_sock_init(sk, NULL); >> bt_sock_link(&l2cap_sk_list, sk); >> return 0; >> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c >> index 1739c1989dbd..25cb5dc580bf 100644 >> --- a/net/bluetooth/smp.c >> +++ b/net/bluetooth/smp.c >> @@ -3204,16 +3204,11 @@ static const struct l2cap_ops smp_chan_ops = { >> .get_sndtimeo = l2cap_chan_no_get_sndtimeo, >> }; >> >> -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) >> +static inline int smp_new_conn_cb(struct l2cap_chan *pchan, >> + struct l2cap_chan *chan) >> { >> - struct l2cap_chan *chan; >> - >> BT_DBG("pchan %p", pchan); >> >> - chan = l2cap_chan_create(); >> - if (!chan) >> - return NULL; >> - >> chan->chan_type = pchan->chan_type; >> chan->ops = &smp_chan_ops; >> chan->scid = pchan->scid; >> @@ -3229,9 +3224,9 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) >> */ >> atomic_set(&chan->nesting, L2CAP_NESTING_SMP); >> >> - BT_DBG("created chan %p", chan); >> + BT_DBG("initialised chan %p", chan); >> >> - return chan; >> + return 0; >> } >> >> static const struct l2cap_ops smp_root_chan_ops = { >> -- >> 2.54.0 >> > > > -- > Luiz Augusto von Dentz Best, Siwei ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-27 4:16 ` Siwei Zhang @ 2026-05-29 16:33 ` Siwei Zhang 2026-05-29 16:40 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 8+ messages in thread From: Siwei Zhang @ 2026-05-29 16:33 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: Marcel Holtmann, linux-bluetooth Hi Luiz, On Wed, May 27, 2026, at 12:16 AM, Siwei Zhang wrote: > Hi Luiz, > > On Tue, May 26, 2026, at 4:46 PM, Luiz Augusto von Dentz wrote: >> Hi Siwei, >> >> On Wed, May 20, 2026 at 12:20 PM Siwei Zhang <oss@fourdim.xyz> wrote: >>> >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after >>> release_sock(parent). Once the parent lock is released, the child >>> socket sk can be freed by another task. >>> >>> Allocate the channel outside the func to prevent this. >>> >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c") >>> Cc: stable@kernel.org >>> Assisted-by: Claude:claude-opus-4-6 >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz> >>> --- >>> include/net/bluetooth/l2cap.h | 8 +++-- >>> net/bluetooth/6lowpan.c | 14 ++++----- >>> net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- >>> net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ >>> net/bluetooth/smp.c | 13 +++----- >>> 5 files changed, 91 insertions(+), 50 deletions(-) >>> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h >>> index 5172afee5494..f7a11e6431f0 100644 >>> --- a/include/net/bluetooth/l2cap.h >>> +++ b/include/net/bluetooth/l2cap.h >>> @@ -619,7 +619,8 @@ struct l2cap_chan { >>> struct l2cap_ops { >>> char *name; >>> >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan); >>> + int (*new_connection)(struct l2cap_chan *chan, >>> + struct l2cap_chan *new_chan); >>> int (*recv) (struct l2cap_chan * chan, >>> struct sk_buff *skb); >>> void (*teardown) (struct l2cap_chan *chan, int err); >>> @@ -883,9 +884,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) >>> return (seq + 1) % (chan->tx_win_max + 1); >>> } >>> >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan) >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_chan *chan, >>> + struct l2cap_chan *new_chan) >>> { >>> - return NULL; >>> + return -EOPNOTSUPP; >>> } >>> >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb) >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c >>> index 23a229ab6a33..286c0b45055b 100644 >>> --- a/net/bluetooth/6lowpan.c >>> +++ b/net/bluetooth/6lowpan.c >>> @@ -743,19 +743,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan) >>> ifup(dev->netdev); >>> } >>> >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan) >>> +static inline int chan_new_conn_cb(struct l2cap_chan *pchan, >>> + struct l2cap_chan *chan) >>> { >>> - struct l2cap_chan *chan; >>> - >>> - chan = chan_create(); >>> - if (!chan) >>> - return NULL; >>> + l2cap_chan_set_defaults(chan); >>> >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL; >>> + chan->imtu = 1280; >> >> The 3 lines above make no sense. >> > > chan_create code in 6lowpan.c > > static struct l2cap_chan *chan_create(void) > { > struct l2cap_chan *chan; > > chan = l2cap_chan_create(); > if (!chan) > return NULL; > > l2cap_chan_set_defaults(chan); > > chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; > chan->mode = L2CAP_MODE_LE_FLOWCTL; > chan->imtu = 1280; > > return chan; > } > > Since we allocate chan outside and replace the chan_create here, > I do think these are needed and they are specific to 6lowpan only. > > I can refactor it in this patch or in a follow-up patch. I would prefer it > to be in a follow-up patch. > What do you think. Please let me know if you want me to change anything. >>> chan->ops = pchan->ops; >>> >>> BT_DBG("chan %p pchan %p", chan, pchan); >>> >>> - return chan; >>> + return 0; >>> } >>> >>> static void unregister_dev(struct lowpan_btle_dev *dev) >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >>> index fdccd62ccca8..505f32034971 100644 >>> --- a/net/bluetooth/l2cap_core.c >>> +++ b/net/bluetooth/l2cap_core.c >>> @@ -4051,10 +4051,16 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, >>> goto response; >>> } >>> >>> - chan = pchan->ops->new_connection(pchan); >>> + chan = l2cap_chan_create(); >>> if (!chan) >>> goto response; >>> >>> + if (pchan->ops->new_connection(pchan, chan) < 0) { >>> + l2cap_chan_put(chan); >>> + chan = NULL; >>> + goto response; >>> + } >>> + >>> /* For certain devices (ex: HID mouse), support for authentication, >>> * pairing and bonding is optional. For such devices, inorder to avoid >>> * the ACL alive for too long after L2CAP disconnection, reset the ACL >>> @@ -4132,6 +4138,10 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, >>> chan->num_conf_req++; >>> } >>> >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >>> + if (chan) >>> + l2cap_chan_put(chan); >>> + >>> l2cap_chan_unlock(pchan); >>> l2cap_chan_put(pchan); >>> } >>> @@ -4881,6 +4891,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >>> struct l2cap_le_conn_rsp rsp; >>> struct l2cap_chan *chan, *pchan; >>> u16 dcid, scid, credits, mtu, mps; >>> + u16 rsp_mtu, rsp_mps; >>> __le16 psm; >>> u8 result; >>> >>> @@ -4893,6 +4904,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >>> psm = req->psm; >>> dcid = 0; >>> credits = 0; >>> + rsp_mtu = 0; >>> + rsp_mps = 0; >>> >>> if (mtu < 23 || mps < 23) >>> return -EPROTO; >>> @@ -4953,12 +4966,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >>> goto response_unlock; >>> } >>> >>> - chan = pchan->ops->new_connection(pchan); >>> + chan = l2cap_chan_create(); >>> if (!chan) { >>> result = L2CAP_CR_LE_NO_MEM; >>> goto response_unlock; >>> } >>> >>> + if (pchan->ops->new_connection(pchan, chan) < 0) { >>> + l2cap_chan_put(chan); >>> + chan = NULL; >>> + result = L2CAP_CR_LE_NO_MEM; >>> + goto response_unlock; >>> + } >>> + >>> bacpy(&chan->src, &conn->hcon->src); >>> bacpy(&chan->dst, &conn->hcon->dst); >>> chan->src_type = bdaddr_src_type(conn->hcon); >>> @@ -4974,6 +4994,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >>> >>> dcid = chan->scid; >>> credits = chan->rx_credits; >>> + rsp_mtu = chan->imtu; >>> + rsp_mps = chan->mps; >>> >>> __set_chan_timer(chan, chan->ops->get_sndtimeo(chan)); >>> >>> @@ -4993,6 +5015,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >>> result = L2CAP_CR_LE_SUCCESS; >>> } >>> >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >>> + l2cap_chan_put(chan); >>> + >>> response_unlock: >>> l2cap_chan_unlock(pchan); >>> l2cap_chan_put(pchan); >>> @@ -5001,13 +5026,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, >>> return 0; >>> >>> response: >>> - if (chan) { >>> - rsp.mtu = cpu_to_le16(chan->imtu); >>> - rsp.mps = cpu_to_le16(chan->mps); >>> - } else { >>> - rsp.mtu = 0; >>> - rsp.mps = 0; >>> - } >>> + rsp.mtu = cpu_to_le16(rsp_mtu); >>> + rsp.mps = cpu_to_le16(rsp_mps); >>> >>> rsp.dcid = cpu_to_le16(dcid); >>> rsp.credits = cpu_to_le16(credits); >>> @@ -5177,12 +5197,18 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, >>> continue; >>> } >>> >>> - chan = pchan->ops->new_connection(pchan); >>> + chan = l2cap_chan_create(); >>> if (!chan) { >>> result = L2CAP_CR_LE_NO_MEM; >>> continue; >>> } >>> >>> + if (pchan->ops->new_connection(pchan, chan) < 0) { >>> + l2cap_chan_put(chan); >>> + result = L2CAP_CR_LE_NO_MEM; >>> + continue; >>> + } >>> + >>> bacpy(&chan->src, &conn->hcon->src); >>> bacpy(&chan->dst, &conn->hcon->dst); >>> chan->src_type = bdaddr_src_type(conn->hcon); >>> @@ -5217,6 +5243,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, >>> } else { >>> l2cap_chan_ready(chan); >>> } >>> + >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >>> + l2cap_chan_put(chan); >>> } >>> >>> unlock: >>> @@ -7399,7 +7428,11 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) >>> goto next; >>> >>> l2cap_chan_lock(pchan); >>> - chan = pchan->ops->new_connection(pchan); >>> + chan = l2cap_chan_create(); >>> + if (chan && pchan->ops->new_connection(pchan, chan) < 0) { >>> + l2cap_chan_put(chan); >>> + chan = NULL; >>> + } >>> if (chan) { >>> bacpy(&chan->src, &hcon->src); >>> bacpy(&chan->dst, &hcon->dst); >>> @@ -7407,6 +7440,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) >>> chan->dst_type = dst_type; >>> >>> __l2cap_chan_add(conn, chan); >>> + >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ >>> + l2cap_chan_put(chan); >>> } >>> >>> l2cap_chan_unlock(pchan); >>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c >>> index dede550d6031..598f24c8f704 100644 >>> --- a/net/bluetooth/l2cap_sock.c >>> +++ b/net/bluetooth/l2cap_sock.c >>> @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = { >>> static const struct proto_ops l2cap_sock_ops; >>> static void l2cap_sock_init(struct sock *sk, struct sock *parent); >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, >>> - int proto, gfp_t prio, int kern); >>> + int proto, gfp_t prio, int kern, >>> + struct l2cap_chan *chan); >>> static void l2cap_sock_cleanup_listen(struct sock *parent); >>> >>> bool l2cap_is_socket(struct socket *sock) >>> @@ -1507,12 +1508,13 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) >>> } >>> } >>> >>> -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) >>> +static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan, >>> + struct l2cap_chan *new_chan) >>> { >>> struct sock *sk, *parent = chan->data; >>> >>> if (!parent) >>> - return NULL; >>> + return -EINVAL; >>> >>> lock_sock(parent); >>> >>> @@ -1520,15 +1522,15 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) >>> if (sk_acceptq_is_full(parent)) { >>> BT_DBG("backlog full %d", parent->sk_ack_backlog); >>> release_sock(parent); >>> - return NULL; >>> + return -ENOBUFS; >>> } >>> >>> sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, >>> - GFP_ATOMIC, 0); >>> + GFP_ATOMIC, 0, new_chan); >>> if (!sk) { >>> release_sock(parent); >>> - return NULL; >>> - } >>> + return -ENOMEM; >>> + } >>> >>> bt_sock_reclassify_lock(sk, BTPROTO_L2CAP); >>> >>> @@ -1538,7 +1540,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) >>> >>> release_sock(parent); >>> >>> - return l2cap_pi(sk)->chan; >>> + return 0; >>> } >>> >>> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) >>> @@ -1939,10 +1941,10 @@ static struct proto l2cap_proto = { >>> }; >>> >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, >>> - int proto, gfp_t prio, int kern) >>> + int proto, gfp_t prio, int kern, >>> + struct l2cap_chan *chan) >>> { >>> struct sock *sk; >>> - struct l2cap_chan *chan; >>> >>> sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern); >>> if (!sk) >>> @@ -1953,14 +1955,11 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, >>> >>> INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy); >>> >>> - chan = l2cap_chan_create(); >>> - if (!chan) { >>> - sk_free(sk); >>> - if (sock) >>> - sock->sk = NULL; >>> - return NULL; >>> - } >>> - >>> + /* The sock owns two refs on chan, matching the puts in >>> + * l2cap_sock_kill() and l2cap_sock_destruct(). The caller keeps >>> + * its own ref independent of these. >>> + */ >>> + l2cap_chan_hold(chan); >>> l2cap_chan_hold(chan); >>> >>> l2cap_pi(sk)->chan = chan; >>> @@ -1972,6 +1971,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, >>> int kern) >>> { >>> struct sock *sk; >>> + struct l2cap_chan *chan; >>> >>> BT_DBG("sock %p", sock); >>> >>> @@ -1986,10 +1986,18 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, >>> >>> sock->ops = &l2cap_sock_ops; >>> >>> - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern); >>> - if (!sk) >>> + chan = l2cap_chan_create(); >>> + if (!chan) >>> return -ENOMEM; >>> >>> + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan); >>> + if (!sk) { >>> + l2cap_chan_put(chan); >>> + return -ENOMEM; >>> + } >>> + /* Sock has taken its own refs on chan; drop the chan_create() ref. */ >>> + l2cap_chan_put(chan); >>> + >>> l2cap_sock_init(sk, NULL); >>> bt_sock_link(&l2cap_sk_list, sk); >>> return 0; >>> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c >>> index 1739c1989dbd..25cb5dc580bf 100644 >>> --- a/net/bluetooth/smp.c >>> +++ b/net/bluetooth/smp.c >>> @@ -3204,16 +3204,11 @@ static const struct l2cap_ops smp_chan_ops = { >>> .get_sndtimeo = l2cap_chan_no_get_sndtimeo, >>> }; >>> >>> -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) >>> +static inline int smp_new_conn_cb(struct l2cap_chan *pchan, >>> + struct l2cap_chan *chan) >>> { >>> - struct l2cap_chan *chan; >>> - >>> BT_DBG("pchan %p", pchan); >>> >>> - chan = l2cap_chan_create(); >>> - if (!chan) >>> - return NULL; >>> - >>> chan->chan_type = pchan->chan_type; >>> chan->ops = &smp_chan_ops; >>> chan->scid = pchan->scid; >>> @@ -3229,9 +3224,9 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) >>> */ >>> atomic_set(&chan->nesting, L2CAP_NESTING_SMP); >>> >>> - BT_DBG("created chan %p", chan); >>> + BT_DBG("initialised chan %p", chan); >>> >>> - return chan; >>> + return 0; >>> } >>> >>> static const struct l2cap_ops smp_root_chan_ops = { >>> -- >>> 2.54.0 >>> >> >> >> -- >> Luiz Augusto von Dentz > > Best, > Siwei Best, Siwei ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() 2026-05-29 16:33 ` Siwei Zhang @ 2026-05-29 16:40 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 8+ messages in thread From: Luiz Augusto von Dentz @ 2026-05-29 16:40 UTC (permalink / raw) To: Siwei Zhang; +Cc: Marcel Holtmann, linux-bluetooth Hi Siwei, On Fri, May 29, 2026 at 12:33 PM Siwei Zhang <oss@fourdim.xyz> wrote: > > Hi Luiz, > > On Wed, May 27, 2026, at 12:16 AM, Siwei Zhang wrote: > > Hi Luiz, > > > > On Tue, May 26, 2026, at 4:46 PM, Luiz Augusto von Dentz wrote: > >> Hi Siwei, > >> > >> On Wed, May 20, 2026 at 12:20 PM Siwei Zhang <oss@fourdim.xyz> wrote: > >>> > >>> l2cap_sock_new_connection_cb() accesses l2cap_pi(sk)->chan after > >>> release_sock(parent). Once the parent lock is released, the child > >>> socket sk can be freed by another task. > >>> > >>> Allocate the channel outside the func to prevent this. > >>> > >>> Fixes: 8ffb929098a5 ("Bluetooth: Remove parent socket usage from l2cap_core.c") > >>> Cc: stable@kernel.org > >>> Assisted-by: Claude:claude-opus-4-6 > >>> Signed-off-by: Siwei Zhang <oss@fourdim.xyz> > >>> --- > >>> include/net/bluetooth/l2cap.h | 8 +++-- > >>> net/bluetooth/6lowpan.c | 14 ++++----- > >>> net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++------- > >>> net/bluetooth/l2cap_sock.c | 48 +++++++++++++++++------------ > >>> net/bluetooth/smp.c | 13 +++----- > >>> 5 files changed, 91 insertions(+), 50 deletions(-) > >>> > >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > >>> index 5172afee5494..f7a11e6431f0 100644 > >>> --- a/include/net/bluetooth/l2cap.h > >>> +++ b/include/net/bluetooth/l2cap.h > >>> @@ -619,7 +619,8 @@ struct l2cap_chan { > >>> struct l2cap_ops { > >>> char *name; > >>> > >>> - struct l2cap_chan *(*new_connection) (struct l2cap_chan *chan); > >>> + int (*new_connection)(struct l2cap_chan *chan, > >>> + struct l2cap_chan *new_chan); > >>> int (*recv) (struct l2cap_chan * chan, > >>> struct sk_buff *skb); > >>> void (*teardown) (struct l2cap_chan *chan, int err); > >>> @@ -883,9 +884,10 @@ static inline __u16 __next_seq(struct l2cap_chan *chan, __u16 seq) > >>> return (seq + 1) % (chan->tx_win_max + 1); > >>> } > >>> > >>> -static inline struct l2cap_chan *l2cap_chan_no_new_connection(struct l2cap_chan *chan) > >>> +static inline int l2cap_chan_no_new_connection(struct l2cap_chan *chan, > >>> + struct l2cap_chan *new_chan) > >>> { > >>> - return NULL; > >>> + return -EOPNOTSUPP; > >>> } > >>> > >>> static inline int l2cap_chan_no_recv(struct l2cap_chan *chan, struct sk_buff *skb) > >>> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c > >>> index 23a229ab6a33..286c0b45055b 100644 > >>> --- a/net/bluetooth/6lowpan.c > >>> +++ b/net/bluetooth/6lowpan.c > >>> @@ -743,19 +743,19 @@ static inline void chan_ready_cb(struct l2cap_chan *chan) > >>> ifup(dev->netdev); > >>> } > >>> > >>> -static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan) > >>> +static inline int chan_new_conn_cb(struct l2cap_chan *pchan, > >>> + struct l2cap_chan *chan) > >>> { > >>> - struct l2cap_chan *chan; > >>> - > >>> - chan = chan_create(); > >>> - if (!chan) > >>> - return NULL; > >>> + l2cap_chan_set_defaults(chan); > >>> > >>> + chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; > >>> + chan->mode = L2CAP_MODE_LE_FLOWCTL; > >>> + chan->imtu = 1280; > >> > >> The 3 lines above make no sense. > >> > > > > chan_create code in 6lowpan.c > > > > static struct l2cap_chan *chan_create(void) > > { > > struct l2cap_chan *chan; > > > > chan = l2cap_chan_create(); > > if (!chan) > > return NULL; > > > > l2cap_chan_set_defaults(chan); > > > > chan->chan_type = L2CAP_CHAN_CONN_ORIENTED; > > chan->mode = L2CAP_MODE_LE_FLOWCTL; > > chan->imtu = 1280; > > > > return chan; > > } > > > > Since we allocate chan outside and replace the chan_create here, > > I do think these are needed and they are specific to 6lowpan only. > > > > I can refactor it in this patch or in a follow-up patch. I would prefer it > > to be in a follow-up patch. > > > > What do you think. Please let me know if you want me to change anything. There is something off then, because nothing is being removed that sets similar values which is what led me to believe it was necessary at first. > >>> chan->ops = pchan->ops; > >>> > >>> BT_DBG("chan %p pchan %p", chan, pchan); > >>> > >>> - return chan; > >>> + return 0; > >>> } > >>> > >>> static void unregister_dev(struct lowpan_btle_dev *dev) > >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > >>> index fdccd62ccca8..505f32034971 100644 > >>> --- a/net/bluetooth/l2cap_core.c > >>> +++ b/net/bluetooth/l2cap_core.c > >>> @@ -4051,10 +4051,16 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, > >>> goto response; > >>> } > >>> > >>> - chan = pchan->ops->new_connection(pchan); > >>> + chan = l2cap_chan_create(); > >>> if (!chan) > >>> goto response; > >>> > >>> + if (pchan->ops->new_connection(pchan, chan) < 0) { > >>> + l2cap_chan_put(chan); > >>> + chan = NULL; > >>> + goto response; > >>> + } > >>> + > >>> /* For certain devices (ex: HID mouse), support for authentication, > >>> * pairing and bonding is optional. For such devices, inorder to avoid > >>> * the ACL alive for too long after L2CAP disconnection, reset the ACL > >>> @@ -4132,6 +4138,10 @@ static void l2cap_connect(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, > >>> chan->num_conf_req++; > >>> } > >>> > >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > >>> + if (chan) > >>> + l2cap_chan_put(chan); > >>> + > >>> l2cap_chan_unlock(pchan); > >>> l2cap_chan_put(pchan); > >>> } > >>> @@ -4881,6 +4891,7 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > >>> struct l2cap_le_conn_rsp rsp; > >>> struct l2cap_chan *chan, *pchan; > >>> u16 dcid, scid, credits, mtu, mps; > >>> + u16 rsp_mtu, rsp_mps; > >>> __le16 psm; > >>> u8 result; > >>> > >>> @@ -4893,6 +4904,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > >>> psm = req->psm; > >>> dcid = 0; > >>> credits = 0; > >>> + rsp_mtu = 0; > >>> + rsp_mps = 0; > >>> > >>> if (mtu < 23 || mps < 23) > >>> return -EPROTO; > >>> @@ -4953,12 +4966,19 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > >>> goto response_unlock; > >>> } > >>> > >>> - chan = pchan->ops->new_connection(pchan); > >>> + chan = l2cap_chan_create(); > >>> if (!chan) { > >>> result = L2CAP_CR_LE_NO_MEM; > >>> goto response_unlock; > >>> } > >>> > >>> + if (pchan->ops->new_connection(pchan, chan) < 0) { > >>> + l2cap_chan_put(chan); > >>> + chan = NULL; > >>> + result = L2CAP_CR_LE_NO_MEM; > >>> + goto response_unlock; > >>> + } > >>> + > >>> bacpy(&chan->src, &conn->hcon->src); > >>> bacpy(&chan->dst, &conn->hcon->dst); > >>> chan->src_type = bdaddr_src_type(conn->hcon); > >>> @@ -4974,6 +4994,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > >>> > >>> dcid = chan->scid; > >>> credits = chan->rx_credits; > >>> + rsp_mtu = chan->imtu; > >>> + rsp_mps = chan->mps; > >>> > >>> __set_chan_timer(chan, chan->ops->get_sndtimeo(chan)); > >>> > >>> @@ -4993,6 +5015,9 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > >>> result = L2CAP_CR_LE_SUCCESS; > >>> } > >>> > >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > >>> + l2cap_chan_put(chan); > >>> + > >>> response_unlock: > >>> l2cap_chan_unlock(pchan); > >>> l2cap_chan_put(pchan); > >>> @@ -5001,13 +5026,8 @@ static int l2cap_le_connect_req(struct l2cap_conn *conn, > >>> return 0; > >>> > >>> response: > >>> - if (chan) { > >>> - rsp.mtu = cpu_to_le16(chan->imtu); > >>> - rsp.mps = cpu_to_le16(chan->mps); > >>> - } else { > >>> - rsp.mtu = 0; > >>> - rsp.mps = 0; > >>> - } > >>> + rsp.mtu = cpu_to_le16(rsp_mtu); > >>> + rsp.mps = cpu_to_le16(rsp_mps); > >>> > >>> rsp.dcid = cpu_to_le16(dcid); > >>> rsp.credits = cpu_to_le16(credits); > >>> @@ -5177,12 +5197,18 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, > >>> continue; > >>> } > >>> > >>> - chan = pchan->ops->new_connection(pchan); > >>> + chan = l2cap_chan_create(); > >>> if (!chan) { > >>> result = L2CAP_CR_LE_NO_MEM; > >>> continue; > >>> } > >>> > >>> + if (pchan->ops->new_connection(pchan, chan) < 0) { > >>> + l2cap_chan_put(chan); > >>> + result = L2CAP_CR_LE_NO_MEM; > >>> + continue; > >>> + } > >>> + > >>> bacpy(&chan->src, &conn->hcon->src); > >>> bacpy(&chan->dst, &conn->hcon->dst); > >>> chan->src_type = bdaddr_src_type(conn->hcon); > >>> @@ -5217,6 +5243,9 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn, > >>> } else { > >>> l2cap_chan_ready(chan); > >>> } > >>> + > >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > >>> + l2cap_chan_put(chan); > >>> } > >>> > >>> unlock: > >>> @@ -7399,7 +7428,11 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > >>> goto next; > >>> > >>> l2cap_chan_lock(pchan); > >>> - chan = pchan->ops->new_connection(pchan); > >>> + chan = l2cap_chan_create(); > >>> + if (chan && pchan->ops->new_connection(pchan, chan) < 0) { > >>> + l2cap_chan_put(chan); > >>> + chan = NULL; > >>> + } > >>> if (chan) { > >>> bacpy(&chan->src, &hcon->src); > >>> bacpy(&chan->dst, &hcon->dst); > >>> @@ -7407,6 +7440,9 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status) > >>> chan->dst_type = dst_type; > >>> > >>> __l2cap_chan_add(conn, chan); > >>> + > >>> + /* Drop our local ref; __l2cap_chan_add() pinned chan via the conn list. */ > >>> + l2cap_chan_put(chan); > >>> } > >>> > >>> l2cap_chan_unlock(pchan); > >>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c > >>> index dede550d6031..598f24c8f704 100644 > >>> --- a/net/bluetooth/l2cap_sock.c > >>> +++ b/net/bluetooth/l2cap_sock.c > >>> @@ -46,7 +46,8 @@ static struct bt_sock_list l2cap_sk_list = { > >>> static const struct proto_ops l2cap_sock_ops; > >>> static void l2cap_sock_init(struct sock *sk, struct sock *parent); > >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > >>> - int proto, gfp_t prio, int kern); > >>> + int proto, gfp_t prio, int kern, > >>> + struct l2cap_chan *chan); > >>> static void l2cap_sock_cleanup_listen(struct sock *parent); > >>> > >>> bool l2cap_is_socket(struct socket *sock) > >>> @@ -1507,12 +1508,13 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) > >>> } > >>> } > >>> > >>> -static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > >>> +static int l2cap_sock_new_connection_cb(struct l2cap_chan *chan, > >>> + struct l2cap_chan *new_chan) > >>> { > >>> struct sock *sk, *parent = chan->data; > >>> > >>> if (!parent) > >>> - return NULL; > >>> + return -EINVAL; > >>> > >>> lock_sock(parent); > >>> > >>> @@ -1520,15 +1522,15 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > >>> if (sk_acceptq_is_full(parent)) { > >>> BT_DBG("backlog full %d", parent->sk_ack_backlog); > >>> release_sock(parent); > >>> - return NULL; > >>> + return -ENOBUFS; > >>> } > >>> > >>> sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, > >>> - GFP_ATOMIC, 0); > >>> + GFP_ATOMIC, 0, new_chan); > >>> if (!sk) { > >>> release_sock(parent); > >>> - return NULL; > >>> - } > >>> + return -ENOMEM; > >>> + } > >>> > >>> bt_sock_reclassify_lock(sk, BTPROTO_L2CAP); > >>> > >>> @@ -1538,7 +1540,7 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan) > >>> > >>> release_sock(parent); > >>> > >>> - return l2cap_pi(sk)->chan; > >>> + return 0; > >>> } > >>> > >>> static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) > >>> @@ -1939,10 +1941,10 @@ static struct proto l2cap_proto = { > >>> }; > >>> > >>> static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > >>> - int proto, gfp_t prio, int kern) > >>> + int proto, gfp_t prio, int kern, > >>> + struct l2cap_chan *chan) > >>> { > >>> struct sock *sk; > >>> - struct l2cap_chan *chan; > >>> > >>> sk = bt_sock_alloc(net, sock, &l2cap_proto, proto, prio, kern); > >>> if (!sk) > >>> @@ -1953,14 +1955,11 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, > >>> > >>> INIT_LIST_HEAD(&l2cap_pi(sk)->rx_busy); > >>> > >>> - chan = l2cap_chan_create(); > >>> - if (!chan) { > >>> - sk_free(sk); > >>> - if (sock) > >>> - sock->sk = NULL; > >>> - return NULL; > >>> - } > >>> - > >>> + /* The sock owns two refs on chan, matching the puts in > >>> + * l2cap_sock_kill() and l2cap_sock_destruct(). The caller keeps > >>> + * its own ref independent of these. > >>> + */ > >>> + l2cap_chan_hold(chan); > >>> l2cap_chan_hold(chan); > >>> > >>> l2cap_pi(sk)->chan = chan; > >>> @@ -1972,6 +1971,7 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, > >>> int kern) > >>> { > >>> struct sock *sk; > >>> + struct l2cap_chan *chan; > >>> > >>> BT_DBG("sock %p", sock); > >>> > >>> @@ -1986,10 +1986,18 @@ static int l2cap_sock_create(struct net *net, struct socket *sock, int protocol, > >>> > >>> sock->ops = &l2cap_sock_ops; > >>> > >>> - sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern); > >>> - if (!sk) > >>> + chan = l2cap_chan_create(); > >>> + if (!chan) > >>> return -ENOMEM; > >>> > >>> + sk = l2cap_sock_alloc(net, sock, protocol, GFP_ATOMIC, kern, chan); > >>> + if (!sk) { > >>> + l2cap_chan_put(chan); > >>> + return -ENOMEM; > >>> + } > >>> + /* Sock has taken its own refs on chan; drop the chan_create() ref. */ > >>> + l2cap_chan_put(chan); > >>> + > >>> l2cap_sock_init(sk, NULL); > >>> bt_sock_link(&l2cap_sk_list, sk); > >>> return 0; > >>> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c > >>> index 1739c1989dbd..25cb5dc580bf 100644 > >>> --- a/net/bluetooth/smp.c > >>> +++ b/net/bluetooth/smp.c > >>> @@ -3204,16 +3204,11 @@ static const struct l2cap_ops smp_chan_ops = { > >>> .get_sndtimeo = l2cap_chan_no_get_sndtimeo, > >>> }; > >>> > >>> -static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) > >>> +static inline int smp_new_conn_cb(struct l2cap_chan *pchan, > >>> + struct l2cap_chan *chan) > >>> { > >>> - struct l2cap_chan *chan; > >>> - > >>> BT_DBG("pchan %p", pchan); > >>> > >>> - chan = l2cap_chan_create(); > >>> - if (!chan) > >>> - return NULL; > >>> - > >>> chan->chan_type = pchan->chan_type; > >>> chan->ops = &smp_chan_ops; > >>> chan->scid = pchan->scid; > >>> @@ -3229,9 +3224,9 @@ static inline struct l2cap_chan *smp_new_conn_cb(struct l2cap_chan *pchan) > >>> */ > >>> atomic_set(&chan->nesting, L2CAP_NESTING_SMP); > >>> > >>> - BT_DBG("created chan %p", chan); > >>> + BT_DBG("initialised chan %p", chan); > >>> > >>> - return chan; > >>> + return 0; > >>> } > >>> > >>> static const struct l2cap_ops smp_root_chan_ops = { > >>> -- > >>> 2.54.0 > >>> > >> > >> > >> -- > >> Luiz Augusto von Dentz > > > > Best, > > Siwei > > Best, > Siwei -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-29 16:40 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-20 16:20 [PATCH v5 0/1] Bluetooth: L2CAP: Fix use-after-free in l2cap_sock_new_connection_cb() Siwei Zhang 2026-05-20 16:20 ` [PATCH v5 1/1] " Siwei Zhang 2026-05-20 18:07 ` bluez.test.bot 2026-05-26 20:38 ` [PATCH v5 1/1] " Siwei Zhang 2026-05-26 20:46 ` Luiz Augusto von Dentz 2026-05-27 4:16 ` Siwei Zhang 2026-05-29 16:33 ` Siwei Zhang 2026-05-29 16:40 ` 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