Re: [RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
Hi Luiz,
> It is possible to receive an L2CAP conn req for an encrypted
> connection, before actually receiving the HCI change encryption
> event. If this happened, the received L2CAP packet will be ignored.
>>>
>>> How is this possible? Or you are referring to a race between the ACL
>>> and Event endpoint where the Encryption Change is actually pending to
>>> be processed but we end up processing the ACL data first.
>>
>> you get the ACL packet with the L2CAP_Connect_Req in it and then the HCI
>> Encryption Change event. However over the air they go in the different
>> order. It is specific to the USB transport and nothing is going to fix this.
>> The USB transport design is borked. You can only do bandaids.
>>
> This patch queues the L2CAP packet and process them after the
> expected HCI event is received. If after 2 seconds we still don't
> receive it, then we assume something bad happened and discard the
> queued packets.
as with the other patch, this should be behind the same quirk and
experimental setting for exactly the same reasons.
>
> Signed-off-by: Archie Pusaka
> Reviewed-by: Abhishek Pandit-Subedi
>
> ---
>
> include/net/bluetooth/bluetooth.h | 6 +++
> include/net/bluetooth/l2cap.h | 6 +++
> net/bluetooth/hci_event.c | 3 ++
> net/bluetooth/l2cap_core.c| 87 +++
> 4 files changed, 91 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h
> b/include/net/bluetooth/bluetooth.h
> index 7ee8041af803..e64278401084 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -335,7 +335,11 @@ struct l2cap_ctrl {
> u16 reqseq;
> u16 txseq;
> u8 retries;
> + u8 rsp_code;
> + u8 amp_id;
> + __u8ident;
> __le16 psm;
> + __le16 scid;
> bdaddr_t bdaddr;
> struct l2cap_chan *chan;
> };
I would not bother trying to make this work with CREATE_CHAN_REQ. That is
if you want to setup a L2CAP channel that can be moved between BR/EDR and
AMP controllers and in that case you have to read the L2CAP information
and features first. Meaning there will have been unencrypted ACL packets.
This problem only exists if the remote side doesn’t request any version
information first.
> @@ -374,6 +378,8 @@ struct bt_skb_cb {
> struct hci_ctrl hci;
> };
> };
> +static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff
> *)0)->cb));
> +
> #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
>
> #define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 8f1e6a7a2df8..f8f6dec96f12 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -58,6 +58,7 @@
> #define L2CAP_MOVE_ERTX_TIMEOUT msecs_to_jiffies(6)
> #define L2CAP_WAIT_ACK_POLL_PERIODmsecs_to_jiffies(200)
> #define L2CAP_WAIT_ACK_TIMEOUTmsecs_to_jiffies(1)
> +#define L2CAP_PEND_ENC_CONN_TIMEOUT msecs_to_jiffies(2000)
>
> #define L2CAP_A2MP_DEFAULT_MTU670
>
> @@ -700,6 +701,9 @@ struct l2cap_conn {
> struct mutexchan_lock;
> struct kref ref;
> struct list_headusers;
> +
> + struct delayed_work remove_pending_encrypt_conn;
> + struct sk_buff_head pending_conn_q;
> };
>
> struct l2cap_user {
> @@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
> int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
> void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user
> *user);
>
> +void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
> +
> #endif /* __L2CAP_H */
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 108c6c102a6a..8cefc51a5ca4 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev
> *hdev, struct sk_buff *skb)
>
> unlock:
> hci_dev_unlock(hdev);
> +
> + if (conn && !ev->status && ev->encrypt)
> + l2cap_process_pending_encrypt_conn(conn);
> }
>
> static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 35d2bc569a2d..fc6fe2c80c46 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -62,6 +62,10 @@ static void l2cap_send_disconn_req(struct l2cap_chan
> *chan, int
Re: [RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
Hi Marcel,
On Mon, Jun 29, 2020 at 1:21 PM Marcel Holtmann wrote:
>
> Hi Luiz,
>
> >>> It is possible to receive an L2CAP conn req for an encrypted
> >>> connection, before actually receiving the HCI change encryption
> >>> event. If this happened, the received L2CAP packet will be ignored.
> >
> > How is this possible? Or you are referring to a race between the ACL
> > and Event endpoint where the Encryption Change is actually pending to
> > be processed but we end up processing the ACL data first.
>
> you get the ACL packet with the L2CAP_Connect_Req in it and then the HCI
> Encryption Change event. However over the air they go in the different order.
> It is specific to the USB transport and nothing is going to fix this. The USB
> transport design is borked. You can only do bandaids.
>
> >>> This patch queues the L2CAP packet and process them after the
> >>> expected HCI event is received. If after 2 seconds we still don't
> >>> receive it, then we assume something bad happened and discard the
> >>> queued packets.
> >>
> >> as with the other patch, this should be behind the same quirk and
> >> experimental setting for exactly the same reasons.
> >>
> >>>
> >>> Signed-off-by: Archie Pusaka
> >>> Reviewed-by: Abhishek Pandit-Subedi
> >>>
> >>> ---
> >>>
> >>> include/net/bluetooth/bluetooth.h | 6 +++
> >>> include/net/bluetooth/l2cap.h | 6 +++
> >>> net/bluetooth/hci_event.c | 3 ++
> >>> net/bluetooth/l2cap_core.c| 87 +++
> >>> 4 files changed, 91 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/include/net/bluetooth/bluetooth.h
> >>> b/include/net/bluetooth/bluetooth.h
> >>> index 7ee8041af803..e64278401084 100644
> >>> --- a/include/net/bluetooth/bluetooth.h
> >>> +++ b/include/net/bluetooth/bluetooth.h
> >>> @@ -335,7 +335,11 @@ struct l2cap_ctrl {
> >>> u16 reqseq;
> >>> u16 txseq;
> >>> u8 retries;
> >>> + u8 rsp_code;
> >>> + u8 amp_id;
> >>> + __u8ident;
> >>> __le16 psm;
> >>> + __le16 scid;
> >>> bdaddr_t bdaddr;
> >>> struct l2cap_chan *chan;
> >>> };
> >>
> >> I would not bother trying to make this work with CREATE_CHAN_REQ. That is
> >> if you want to setup a L2CAP channel that can be moved between BR/EDR and
> >> AMP controllers and in that case you have to read the L2CAP information
> >> and features first. Meaning there will have been unencrypted ACL packets.
> >> This problem only exists if the remote side doesn’t request any version
> >> information first.
> >>
> >>> @@ -374,6 +378,8 @@ struct bt_skb_cb {
> >>> struct hci_ctrl hci;
> >>> };
> >>> };
> >>> +static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff
> >>> *)0)->cb));
> >>> +
> >>> #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
> >>>
> >>> #define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
> >>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >>> index 8f1e6a7a2df8..f8f6dec96f12 100644
> >>> --- a/include/net/bluetooth/l2cap.h
> >>> +++ b/include/net/bluetooth/l2cap.h
> >>> @@ -58,6 +58,7 @@
> >>> #define L2CAP_MOVE_ERTX_TIMEOUT msecs_to_jiffies(6)
> >>> #define L2CAP_WAIT_ACK_POLL_PERIODmsecs_to_jiffies(200)
> >>> #define L2CAP_WAIT_ACK_TIMEOUTmsecs_to_jiffies(1)
> >>> +#define L2CAP_PEND_ENC_CONN_TIMEOUT msecs_to_jiffies(2000)
> >>>
> >>> #define L2CAP_A2MP_DEFAULT_MTU670
> >>>
> >>> @@ -700,6 +701,9 @@ struct l2cap_conn {
> >>> struct mutexchan_lock;
> >>> struct kref ref;
> >>> struct list_headusers;
> >>> +
> >>> + struct delayed_work remove_pending_encrypt_conn;
> >>> + struct sk_buff_head pending_conn_q;
> >>> };
> >>>
> >>> struct l2cap_user {
> >>> @@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
> >>> int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
> >>> void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user
> >>> *user);
> >>>
> >>> +void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
> >>> +
> >>> #endif /* __L2CAP_H */
> >>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> >>> index 108c6c102a6a..8cefc51a5ca4 100644
> >>> --- a/net/bluetooth/hci_event.c
> >>> +++ b/net/bluetooth/hci_event.c
> >>> @@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev
> >>> *hdev, struct sk_buff *skb)
> >>>
> >>> unlock:
> >>> hci_dev_unlock(hdev);
> >>> +
> >>> + if (conn && !ev->status && ev->encrypt)
> >>> + l2cap_process_pending_encrypt_conn(conn);
> >>> }
> >>>
> >>> static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>> index 35d2bc569a2d..fc6fe2c80c46 100644
> >>> --- a/net/bluetooth/l2cap_core.c
> >>> +++ b/net/bluetooth/l2cap_core.c
> >>> @@ -62,6 +62,10 @@ static void
Re: [RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
Hi Luiz,
>>> It is possible to receive an L2CAP conn req for an encrypted
>>> connection, before actually receiving the HCI change encryption
>>> event. If this happened, the received L2CAP packet will be ignored.
>
> How is this possible? Or you are referring to a race between the ACL
> and Event endpoint where the Encryption Change is actually pending to
> be processed but we end up processing the ACL data first.
you get the ACL packet with the L2CAP_Connect_Req in it and then the HCI
Encryption Change event. However over the air they go in the different order.
It is specific to the USB transport and nothing is going to fix this. The USB
transport design is borked. You can only do bandaids.
>>> This patch queues the L2CAP packet and process them after the
>>> expected HCI event is received. If after 2 seconds we still don't
>>> receive it, then we assume something bad happened and discard the
>>> queued packets.
>>
>> as with the other patch, this should be behind the same quirk and
>> experimental setting for exactly the same reasons.
>>
>>>
>>> Signed-off-by: Archie Pusaka
>>> Reviewed-by: Abhishek Pandit-Subedi
>>>
>>> ---
>>>
>>> include/net/bluetooth/bluetooth.h | 6 +++
>>> include/net/bluetooth/l2cap.h | 6 +++
>>> net/bluetooth/hci_event.c | 3 ++
>>> net/bluetooth/l2cap_core.c| 87 +++
>>> 4 files changed, 91 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/bluetooth.h
>>> b/include/net/bluetooth/bluetooth.h
>>> index 7ee8041af803..e64278401084 100644
>>> --- a/include/net/bluetooth/bluetooth.h
>>> +++ b/include/net/bluetooth/bluetooth.h
>>> @@ -335,7 +335,11 @@ struct l2cap_ctrl {
>>> u16 reqseq;
>>> u16 txseq;
>>> u8 retries;
>>> + u8 rsp_code;
>>> + u8 amp_id;
>>> + __u8ident;
>>> __le16 psm;
>>> + __le16 scid;
>>> bdaddr_t bdaddr;
>>> struct l2cap_chan *chan;
>>> };
>>
>> I would not bother trying to make this work with CREATE_CHAN_REQ. That is if
>> you want to setup a L2CAP channel that can be moved between BR/EDR and AMP
>> controllers and in that case you have to read the L2CAP information and
>> features first. Meaning there will have been unencrypted ACL packets. This
>> problem only exists if the remote side doesn’t request any version
>> information first.
>>
>>> @@ -374,6 +378,8 @@ struct bt_skb_cb {
>>> struct hci_ctrl hci;
>>> };
>>> };
>>> +static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff
>>> *)0)->cb));
>>> +
>>> #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
>>>
>>> #define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
>>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>>> index 8f1e6a7a2df8..f8f6dec96f12 100644
>>> --- a/include/net/bluetooth/l2cap.h
>>> +++ b/include/net/bluetooth/l2cap.h
>>> @@ -58,6 +58,7 @@
>>> #define L2CAP_MOVE_ERTX_TIMEOUT msecs_to_jiffies(6)
>>> #define L2CAP_WAIT_ACK_POLL_PERIODmsecs_to_jiffies(200)
>>> #define L2CAP_WAIT_ACK_TIMEOUTmsecs_to_jiffies(1)
>>> +#define L2CAP_PEND_ENC_CONN_TIMEOUT msecs_to_jiffies(2000)
>>>
>>> #define L2CAP_A2MP_DEFAULT_MTU670
>>>
>>> @@ -700,6 +701,9 @@ struct l2cap_conn {
>>> struct mutexchan_lock;
>>> struct kref ref;
>>> struct list_headusers;
>>> +
>>> + struct delayed_work remove_pending_encrypt_conn;
>>> + struct sk_buff_head pending_conn_q;
>>> };
>>>
>>> struct l2cap_user {
>>> @@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
>>> int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
>>> void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user
>>> *user);
>>>
>>> +void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
>>> +
>>> #endif /* __L2CAP_H */
>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>> index 108c6c102a6a..8cefc51a5ca4 100644
>>> --- a/net/bluetooth/hci_event.c
>>> +++ b/net/bluetooth/hci_event.c
>>> @@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev
>>> *hdev, struct sk_buff *skb)
>>>
>>> unlock:
>>> hci_dev_unlock(hdev);
>>> +
>>> + if (conn && !ev->status && ev->encrypt)
>>> + l2cap_process_pending_encrypt_conn(conn);
>>> }
>>>
>>> static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
>>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>>> index 35d2bc569a2d..fc6fe2c80c46 100644
>>> --- a/net/bluetooth/l2cap_core.c
>>> +++ b/net/bluetooth/l2cap_core.c
>>> @@ -62,6 +62,10 @@ static void l2cap_send_disconn_req(struct l2cap_chan
>>> *chan, int err);
>>> static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
>>> struct sk_buff_head *skbs, u8 event);
>>>
>>> +static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
>>> +
Re: [RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
Hi Marcel, Archie,
On Mon, Jun 29, 2020 at 12:00 PM Marcel Holtmann wrote:
>
> Hi Archie,
>
> > It is possible to receive an L2CAP conn req for an encrypted
> > connection, before actually receiving the HCI change encryption
> > event. If this happened, the received L2CAP packet will be ignored.
How is this possible? Or you are referring to a race between the ACL
and Event endpoint where the Encryption Change is actually pending to
be processed but we end up processing the ACL data first.
> > This patch queues the L2CAP packet and process them after the
> > expected HCI event is received. If after 2 seconds we still don't
> > receive it, then we assume something bad happened and discard the
> > queued packets.
>
> as with the other patch, this should be behind the same quirk and
> experimental setting for exactly the same reasons.
>
> >
> > Signed-off-by: Archie Pusaka
> > Reviewed-by: Abhishek Pandit-Subedi
> >
> > ---
> >
> > include/net/bluetooth/bluetooth.h | 6 +++
> > include/net/bluetooth/l2cap.h | 6 +++
> > net/bluetooth/hci_event.c | 3 ++
> > net/bluetooth/l2cap_core.c| 87 +++
> > 4 files changed, 91 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h
> > b/include/net/bluetooth/bluetooth.h
> > index 7ee8041af803..e64278401084 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -335,7 +335,11 @@ struct l2cap_ctrl {
> > u16 reqseq;
> > u16 txseq;
> > u8 retries;
> > + u8 rsp_code;
> > + u8 amp_id;
> > + __u8ident;
> > __le16 psm;
> > + __le16 scid;
> > bdaddr_t bdaddr;
> > struct l2cap_chan *chan;
> > };
>
> I would not bother trying to make this work with CREATE_CHAN_REQ. That is if
> you want to setup a L2CAP channel that can be moved between BR/EDR and AMP
> controllers and in that case you have to read the L2CAP information and
> features first. Meaning there will have been unencrypted ACL packets. This
> problem only exists if the remote side doesn’t request any version
> information first.
>
> > @@ -374,6 +378,8 @@ struct bt_skb_cb {
> > struct hci_ctrl hci;
> > };
> > };
> > +static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff
> > *)0)->cb));
> > +
> > #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
> >
> > #define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index 8f1e6a7a2df8..f8f6dec96f12 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -58,6 +58,7 @@
> > #define L2CAP_MOVE_ERTX_TIMEOUT msecs_to_jiffies(6)
> > #define L2CAP_WAIT_ACK_POLL_PERIODmsecs_to_jiffies(200)
> > #define L2CAP_WAIT_ACK_TIMEOUTmsecs_to_jiffies(1)
> > +#define L2CAP_PEND_ENC_CONN_TIMEOUT msecs_to_jiffies(2000)
> >
> > #define L2CAP_A2MP_DEFAULT_MTU670
> >
> > @@ -700,6 +701,9 @@ struct l2cap_conn {
> > struct mutexchan_lock;
> > struct kref ref;
> > struct list_headusers;
> > +
> > + struct delayed_work remove_pending_encrypt_conn;
> > + struct sk_buff_head pending_conn_q;
> > };
> >
> > struct l2cap_user {
> > @@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
> > int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
> > void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user
> > *user);
> >
> > +void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
> > +
> > #endif /* __L2CAP_H */
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 108c6c102a6a..8cefc51a5ca4 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev
> > *hdev, struct sk_buff *skb)
> >
> > unlock:
> > hci_dev_unlock(hdev);
> > +
> > + if (conn && !ev->status && ev->encrypt)
> > + l2cap_process_pending_encrypt_conn(conn);
> > }
> >
> > static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 35d2bc569a2d..fc6fe2c80c46 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -62,6 +62,10 @@ static void l2cap_send_disconn_req(struct l2cap_chan
> > *chan, int err);
> > static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
> >struct sk_buff_head *skbs, u8 event);
> >
> > +static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
> > + u8 ident, u8 *data, u8 rsp_code,
> > + u8 amp_id, bool queue_if_fail);
> > +
> > static inline u8 bdaddr_type(u8 link_type, u8 bdaddr_type)
> > {
> > if
Re: [RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
Hi Archie,
> It is possible to receive an L2CAP conn req for an encrypted
> connection, before actually receiving the HCI change encryption
> event. If this happened, the received L2CAP packet will be ignored.
>
> This patch queues the L2CAP packet and process them after the
> expected HCI event is received. If after 2 seconds we still don't
> receive it, then we assume something bad happened and discard the
> queued packets.
as with the other patch, this should be behind the same quirk and experimental
setting for exactly the same reasons.
>
> Signed-off-by: Archie Pusaka
> Reviewed-by: Abhishek Pandit-Subedi
>
> ---
>
> include/net/bluetooth/bluetooth.h | 6 +++
> include/net/bluetooth/l2cap.h | 6 +++
> net/bluetooth/hci_event.c | 3 ++
> net/bluetooth/l2cap_core.c| 87 +++
> 4 files changed, 91 insertions(+), 11 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h
> b/include/net/bluetooth/bluetooth.h
> index 7ee8041af803..e64278401084 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -335,7 +335,11 @@ struct l2cap_ctrl {
> u16 reqseq;
> u16 txseq;
> u8 retries;
> + u8 rsp_code;
> + u8 amp_id;
> + __u8ident;
> __le16 psm;
> + __le16 scid;
> bdaddr_t bdaddr;
> struct l2cap_chan *chan;
> };
I would not bother trying to make this work with CREATE_CHAN_REQ. That is if
you want to setup a L2CAP channel that can be moved between BR/EDR and AMP
controllers and in that case you have to read the L2CAP information and
features first. Meaning there will have been unencrypted ACL packets. This
problem only exists if the remote side doesn’t request any version information
first.
> @@ -374,6 +378,8 @@ struct bt_skb_cb {
> struct hci_ctrl hci;
> };
> };
> +static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff *)0)->cb));
> +
> #define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
>
> #define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 8f1e6a7a2df8..f8f6dec96f12 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -58,6 +58,7 @@
> #define L2CAP_MOVE_ERTX_TIMEOUT msecs_to_jiffies(6)
> #define L2CAP_WAIT_ACK_POLL_PERIODmsecs_to_jiffies(200)
> #define L2CAP_WAIT_ACK_TIMEOUTmsecs_to_jiffies(1)
> +#define L2CAP_PEND_ENC_CONN_TIMEOUT msecs_to_jiffies(2000)
>
> #define L2CAP_A2MP_DEFAULT_MTU670
>
> @@ -700,6 +701,9 @@ struct l2cap_conn {
> struct mutexchan_lock;
> struct kref ref;
> struct list_headusers;
> +
> + struct delayed_work remove_pending_encrypt_conn;
> + struct sk_buff_head pending_conn_q;
> };
>
> struct l2cap_user {
> @@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
> int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
> void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user);
>
> +void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
> +
> #endif /* __L2CAP_H */
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 108c6c102a6a..8cefc51a5ca4 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev
> *hdev, struct sk_buff *skb)
>
> unlock:
> hci_dev_unlock(hdev);
> +
> + if (conn && !ev->status && ev->encrypt)
> + l2cap_process_pending_encrypt_conn(conn);
> }
>
> static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 35d2bc569a2d..fc6fe2c80c46 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -62,6 +62,10 @@ static void l2cap_send_disconn_req(struct l2cap_chan
> *chan, int err);
> static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
>struct sk_buff_head *skbs, u8 event);
>
> +static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
> + u8 ident, u8 *data, u8 rsp_code,
> + u8 amp_id, bool queue_if_fail);
> +
> static inline u8 bdaddr_type(u8 link_type, u8 bdaddr_type)
> {
> if (link_type == LE_LINK) {
> @@ -1902,6 +1906,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int
> err)
> if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
> cancel_delayed_work_sync(>info_timer);
>
> + cancel_delayed_work_sync(>remove_pending_encrypt_conn);
> +
> hcon->l2cap_data = NULL;
> conn->hchan = NULL;
> l2cap_conn_put(conn);
> @@ -2023,6 +2029,55 @@ static void l2cap_retrans_timeout(struct work_struct
> *work)
> l2cap_chan_put(chan);
> }
[RFC PATCH v1 2/2] Bluetooth: queue L2CAP conn req if encryption is needed
From: Archie Pusaka
It is possible to receive an L2CAP conn req for an encrypted
connection, before actually receiving the HCI change encryption
event. If this happened, the received L2CAP packet will be ignored.
This patch queues the L2CAP packet and process them after the
expected HCI event is received. If after 2 seconds we still don't
receive it, then we assume something bad happened and discard the
queued packets.
Signed-off-by: Archie Pusaka
Reviewed-by: Abhishek Pandit-Subedi
---
include/net/bluetooth/bluetooth.h | 6 +++
include/net/bluetooth/l2cap.h | 6 +++
net/bluetooth/hci_event.c | 3 ++
net/bluetooth/l2cap_core.c| 87 +++
4 files changed, 91 insertions(+), 11 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h
b/include/net/bluetooth/bluetooth.h
index 7ee8041af803..e64278401084 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -335,7 +335,11 @@ struct l2cap_ctrl {
u16 reqseq;
u16 txseq;
u8 retries;
+ u8 rsp_code;
+ u8 amp_id;
+ __u8ident;
__le16 psm;
+ __le16 scid;
bdaddr_t bdaddr;
struct l2cap_chan *chan;
};
@@ -374,6 +378,8 @@ struct bt_skb_cb {
struct hci_ctrl hci;
};
};
+static_assert(sizeof(struct bt_skb_cb) <= sizeof(((struct sk_buff *)0)->cb));
+
#define bt_cb(skb) ((struct bt_skb_cb *)((skb)->cb))
#define hci_skb_pkt_type(skb) bt_cb((skb))->pkt_type
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 8f1e6a7a2df8..f8f6dec96f12 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -58,6 +58,7 @@
#define L2CAP_MOVE_ERTX_TIMEOUTmsecs_to_jiffies(6)
#define L2CAP_WAIT_ACK_POLL_PERIOD msecs_to_jiffies(200)
#define L2CAP_WAIT_ACK_TIMEOUT msecs_to_jiffies(1)
+#define L2CAP_PEND_ENC_CONN_TIMEOUTmsecs_to_jiffies(2000)
#define L2CAP_A2MP_DEFAULT_MTU 670
@@ -700,6 +701,9 @@ struct l2cap_conn {
struct mutexchan_lock;
struct kref ref;
struct list_headusers;
+
+ struct delayed_work remove_pending_encrypt_conn;
+ struct sk_buff_head pending_conn_q;
};
struct l2cap_user {
@@ -1001,4 +1005,6 @@ void l2cap_conn_put(struct l2cap_conn *conn);
int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user);
+void l2cap_process_pending_encrypt_conn(struct hci_conn *hcon);
+
#endif /* __L2CAP_H */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 108c6c102a6a..8cefc51a5ca4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3136,6 +3136,9 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev,
struct sk_buff *skb)
unlock:
hci_dev_unlock(hdev);
+
+ if (conn && !ev->status && ev->encrypt)
+ l2cap_process_pending_encrypt_conn(conn);
}
static void hci_change_link_key_complete_evt(struct hci_dev *hdev,
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 35d2bc569a2d..fc6fe2c80c46 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -62,6 +62,10 @@ static void l2cap_send_disconn_req(struct l2cap_chan *chan,
int err);
static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
struct sk_buff_head *skbs, u8 event);
+static struct l2cap_chan *l2cap_connect(struct l2cap_conn *conn,
+ u8 ident, u8 *data, u8 rsp_code,
+ u8 amp_id, bool queue_if_fail);
+
static inline u8 bdaddr_type(u8 link_type, u8 bdaddr_type)
{
if (link_type == LE_LINK) {
@@ -1902,6 +1906,8 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
cancel_delayed_work_sync(>info_timer);
+ cancel_delayed_work_sync(>remove_pending_encrypt_conn);
+
hcon->l2cap_data = NULL;
conn->hchan = NULL;
l2cap_conn_put(conn);
@@ -2023,6 +2029,55 @@ static void l2cap_retrans_timeout(struct work_struct
*work)
l2cap_chan_put(chan);
}
+static void l2cap_add_pending_encrypt_conn(struct l2cap_conn *conn,
+ struct l2cap_conn_req *req,
+ u8 ident, u8 rsp_code, u8 amp_id)
+{
+ struct sk_buff *skb = bt_skb_alloc(0, GFP_KERNEL);
+
+ bt_cb(skb)->l2cap.psm = req->psm;
+ bt_cb(skb)->l2cap.scid = req->scid;
+ bt_cb(skb)->l2cap.ident = ident;
+ bt_cb(skb)->l2cap.rsp_code = rsp_code;
+ bt_cb(skb)->l2cap.amp_id = amp_id;
+
+ skb_queue_tail(>pending_conn_q, skb);
+ queue_delayed_work(conn->hcon->hdev->workqueue,
+
