L2CAP updates for FCS, valid PSMs, and stream recv
Subsystems (3)
v4 cover L2CAP updates for FCS, valid PSMs, and stream recv
The previous version of this patch set was "[PATCH v3 0/9] Bluetooth:
L2CAP updates for PSM validation and ERTM" - four patches were already merged, and this group of patches incorporates additional feedback from the v3 iteration.
PATCH v4 1/5 Bluetooth: Only enable L2CAP FCS for ERTM or streaming.
This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16 with L2CAP modes other than ERTM or streaming. At present, this only affects the FCS value shown with getsockopt() for basic mode.
--- net/bluetooth/l2cap.c | 19 +++++++++++++------ 1 files changed, 13 insertions(+), 6 deletions(-)
net/bluetooth/l2cap.c +13 -6
@@ -3071,6 +3071,17 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdreturn 0 ; } +static inline void set_default_fcs (struct l2cap_pinfo *pi) +{ +/* FCS is enabled only in ERTM or streaming mode, if one or both +* sides request it. +*/ +if (pi->mode !=L2CAP_MODE_ERTM && pi->mode !=L2CAP_MODE_STREAMING ) + pi->fcs =L2CAP_FCS_NONE ; +else if (!(pi->conf_state &L2CAP_CONF_NO_FCS_RECV )) + pi->fcs =L2CAP_FCS_CRC16 ; +} +static inline int l2cap_config_req (struct l2cap_conn *conn,struct l2cap_cmd_hdr *cmd,u16 cmd_len,u8 *data) {struct l2cap_conf_req *req = (struct l2cap_conf_req *) data;
@@ -3135,9 +3146,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr goto unlock;if (l2cap_pi (sk)->conf_state &L2CAP_CONF_INPUT_DONE ) { -if (!(l2cap_pi (sk)->conf_state &L2CAP_CONF_NO_FCS_RECV ) || -l2cap_pi (sk)->fcs !=L2CAP_FCS_NONE ) -l2cap_pi (sk)->fcs =L2CAP_FCS_CRC16 ; +set_default_fcs (l2cap_pi (sk)); sk->sk_state =BT_CONNECTED ;
@@ -3225,9 +3234,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdrl2cap_pi (sk)->conf_state |=L2CAP_CONF_INPUT_DONE ;if (l2cap_pi (sk)->conf_state &L2CAP_CONF_OUTPUT_DONE ) { -if (!(l2cap_pi (sk)->conf_state &L2CAP_CONF_NO_FCS_RECV ) || -l2cap_pi (sk)->fcs !=L2CAP_FCS_NONE ) -l2cap_pi (sk)->fcs =L2CAP_FCS_CRC16 ; +set_default_fcs (l2cap_pi (sk)); sk->sk_state =BT_CONNECTED ;l2cap_pi (sk)->next_tx_seq =0 ;
Signature · 6 lines
1.7.1 -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Marcel Holtmann
maintainerRe: [PATCH 1/5] Bluetooth: Only enable L2CAP FCS for ERTM or streaming.
Hi Mat,
Quoted 8 lines
This fixes a bug which caused the FCS setting to show
L2CAP_FCS_CRC16with L2CAP modes other than ERTM or streaming. At present, this only affects the FCS value shown withgetsockopt()for basic mode.Signed-off-by Mat Martineau <[email protected]>--- net/bluetooth/l2cap.c | 19 +++++++++++++------ 1 files changed, 13 insertions(+), 6 deletions(-)
patch has been applied, but please stop adding a . at the end of your subject line ;)
Regards
Marcel
PATCH v4 2/5 Bluetooth: Validate PSM values in calls to connect() and bind().
In reply to Mat MartineauValid L2CAP PSMs are odd numbers, and the least significant bit of the most significant byte must be 0.
--- net/bluetooth/l2cap.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
net/bluetooth/l2cap.c +19 -4
@@ -1008,10 +1008,19 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen) goto done; } -if (la.l2_psm &&__le16_to_cpu (la.l2_psm) <0x1001 && - !capable (CAP_NET_BIND_SERVICE )) { - err = -EACCES ; - goto done; +/* If specified, PSM must be odd and lsb of upper byte must be 0 */ +if (la.l2_psm) { +__u16 psm =__le16_to_cpu (la.l2_psm); + +if ((psm &0x0101 ) !=0x0001 ) { + err = -EINVAL ; + goto done; + } + +if (psm <0x1001 && !capable (CAP_NET_BIND_SERVICE )) { + err = -EACCES ; + goto done; + } }write_lock_bh (&l2cap_sk_list.lock);
@@ -1190,6 +1199,12 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al goto done; } +/* PSM must be odd and lsb of upper byte must be 0 */ +if ((__le16_to_cpu (la.l2_psm) &0x0101 ) !=0x0001 ) { + err = -EINVAL ; + goto done; + } +/* Set destination address and psm */ bacpy (&bt_sk (sk)->dst, &la.l2_bdaddr);l2cap_pi (sk)->psm = la.l2_psm;
Signature · 6 lines
1.7.1 -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Marcel Holtmann
maintainerRe: [PATCH 2/5] Bluetooth: Validate PSM values in calls to connect() and bind().
Hi Mat,
Quoted 14 lines
Valid L2CAP PSMs are odd numbers, and the least significant bit of the most significant byte must be 0.
Signed-off-by Mat Martineau <[email protected]>--- net/bluetooth/l2cap.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)net/bluetooth/l2cap.c +0
-0@@ -1008,10 +1008,19 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen) goto done;
net/bluetooth/l2cap.c static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen) · line 1014
Quoted 10 lines
net/bluetooth/l2cap.c +4
-4} -if (la.l2_psm &&__le16_to_cpu (la.l2_psm) <0x1001 && - !capable (CAP_NET_BIND_SERVICE )) { - err = -EACCES ; - goto done; +/* If specified, PSM must be odd and lsb of upper byte must be 0 */ +if (la.l2_psm) { +__u16 psm =__le16_to_cpu (la.l2_psm); +
I would just put the comment here:
/* PSM must be odd and lsb of upper byte must be 0 */
net/bluetooth/l2cap.c static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen) · line 1018
Quoted 4 lines
net/bluetooth/l2cap.c +4
-0+if ((psm &0x0101 ) !=0x0001 ) { + err = -EINVAL ; + goto done; + }
And then while you at it:
/* Restrict usage of well known PSMs */
net/bluetooth/l2cap.c static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen) · line 1024
Quoted 5 lines
net/bluetooth/l2cap.c +4
-0+if (psm <0x1001 && !capable (CAP_NET_BIND_SERVICE )) { + err = -EACCES ; + goto done; + } }
Maybe there is actually a second place for adding that comment.
Otherwise I am fine with this, but since it is not a regression, this will be only taking in the next tree.
Regards
Marcel
PATCH v4 3/5 Bluetooth: Add common code for stream-oriented recvmsg()
In reply to Mat MartineauThis commit adds a bt_sock_stream_recvmsg() function for use by any Bluetooth code that uses SOCK_STREAM sockets. This code is copied from rfcomm_sock_recvmsg() with minimal modifications to remove RFCOMM-specific functionality and improve readability.
L2CAP (with the SOCK_STREAM socket type) and RFCOMM have common needs when it comes to reading data. Proper stream read semantics require that applications can read from a stream one byte at a time and not lose any data. The RFCOMM code already operated on and pulled data from the underlying L2CAP socket, so very few changes were required to make the code more generic for use with non-RFCOMM data over L2CAP.
Applications that need more awareness of L2CAP frame boundaries are still free to use SOCK_SEQPACKET sockets, and may verify that they connection did not fall back to basic mode by calling getsockopt().
--- include/net/bluetooth/bluetooth.h | 2 + net/bluetooth/af_bluetooth.c | 109 +++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 0 deletions(-)
include/net/bluetooth/bluetooth.h +2 -0
@@ -126,6 +126,8 @@ int bt_sock_unregister(int proto);void bt_sock_link (struct bt_sock_list *l,struct sock *s);void bt_sock_unlink (struct bt_sock_list *l,struct sock *s);int bt_sock_recvmsg (struct kiocb *iocb,struct socket *sock,struct msghdr *msg,size_t len,int flags); +int bt_sock_stream_recvmsg (struct kiocb *iocb,struct socket *sock, +struct msghdr *msg,size_t len,int flags);uint bt_sock_poll (struct file * file,struct socket *sock,poll_table *wait);int bt_sock_ioctl (struct socket *sock,unsigned int cmd,unsigned long arg);int bt_sock_wait_state (struct sock *sk,int state,unsigned long timeo);
net/bluetooth/af_bluetooth.c +109 -0
115 lines not on this page · raw message
Signature · 6 lines
1.7.1 -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
PATCH v4 4/5 Bluetooth: Use common SOCK_STREAM receive code in RFCOMM.
In reply to Mat MartineauTo reduce code duplication, have rfcomm_sock_recvmsg() call bt_sock_stream_recvmsg(). The common bt_sock_stream_recvmsg() code is nearly identical, with the RFCOMM-specific functionality for deferred setup and connection unthrottling left in rfcomm_sock_recvmsg().
--- net/bluetooth/rfcomm/sock.c | 104 +++---------------------------------------- 1 files changed, 6 insertions(+), 98 deletions(-)
net/bluetooth/rfcomm/sock.c +6 -98
@@ -617,121 +617,29 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,return sent; } -static long rfcomm_sock_data_wait (struct sock *sk,long timeo) -{ -DECLARE_WAITQUEUE (wait, current); - -add_wait_queue (sk_sleep (sk), &wait); -for (;;) { -set_current_state (TASK_INTERRUPTIBLE ); - -if (!skb_queue_empty (&sk->sk_receive_queue) || - sk->sk_err || - (sk->sk_shutdown &RCV_SHUTDOWN ) || -signal_pending (current) || - !timeo) -break ; - -set_bit (SOCK_ASYNC_WAITDATA , &sk->sk_socket->flags); -release_sock (sk); - timeo =schedule_timeout (timeo); -lock_sock (sk); -clear_bit (SOCK_ASYNC_WAITDATA , &sk->sk_socket->flags); - } - -__set_current_state (TASK_RUNNING ); -remove_wait_queue (sk_sleep (sk), &wait); -return timeo; -} -static int rfcomm_sock_recvmsg (struct kiocb *iocb,struct socket *sock,struct msghdr *msg,size_t size,int flags) {struct sock *sk = sock->sk;struct rfcomm_dlc *d =rfcomm_pi (sk)->dlc; -int err =0 ; -size_t target, copied =0 ; -long timeo; +int len;if (test_and_clear_bit (RFCOMM_DEFER_SETUP , &d->flags)) {rfcomm_dlc_accept (d);return 0 ; } -if (flags &MSG_OOB ) -return -EOPNOTSUPP ; - - msg->msg_namelen =0 ; - -BT_DBG ("sk %p size %zu" , sk, size); + len =bt_sock_stream_recvmsg (iocb, sock, msg, size, flags);lock_sock (sk); +if (!(flags &MSG_PEEK ) && len >0 ) +atomic_sub (len, &sk->sk_rmem_alloc); - target =sock_rcvlowat (sk, flags &MSG_WAITALL , size); - timeo =sock_rcvtimeo (sk, flags &MSG_DONTWAIT ); - -do { -struct sk_buff *skb; -int chunk; - - skb =skb_dequeue (&sk->sk_receive_queue); -if (!skb) { -if (copied >= target) -break ; - -if ((err =sock_error (sk)) !=0 ) -break ; -if (sk->sk_shutdown &RCV_SHUTDOWN ) -break ; - - err = -EAGAIN ; -if (!timeo) -break ; - - timeo =rfcomm_sock_data_wait (sk, timeo); - -if (signal_pending (current)) { - err =sock_intr_errno (timeo); - goto out; - } -continue ; - } - - chunk = min_t(unsigned int, skb->len, size); -if (memcpy_toiovec (msg->msg_iov, skb->data, chunk)) { -skb_queue_head (&sk->sk_receive_queue, skb); -if (!copied) - copied = -EFAULT ; -break ; - } - copied += chunk; - size -= chunk; - -sock_recv_ts_and_drops (msg, sk, skb); - -if (!(flags &MSG_PEEK )) { -atomic_sub (chunk, &sk->sk_rmem_alloc); - -skb_pull (skb, chunk); -if (skb->len) { -skb_queue_head (&sk->sk_receive_queue, skb); -break ; - } -kfree_skb (skb); - - }else { -/* put message back and return */ -skb_queue_head (&sk->sk_receive_queue, skb); -break ; - } - }while (size); - -out:if (atomic_read (&sk->sk_rmem_alloc) <= (sk->sk_rcvbuf >>2 ))rfcomm_dlc_unthrottle (rfcomm_pi (sk)->dlc); -release_sock (sk); -return copied ? : err; + +return len; }static int rfcomm_sock_setsockopt_old (struct socket *sock,int optname,char __user *optval,unsigned int optlen)
Signature · 6 lines
1.7.1 -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
PATCH v4 5/5 Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
In reply to Mat MartineauL2CAP ERTM sockets can be opened with the SOCK_STREAM socket type, which is a mandatory request for ERTM mode.
However, these sockets still have SOCK_SEQPACKET read semantics when bt_sock_recvmsg() is used to pull data from the receive queue. If the application is only reading part of a frame, then the unread portion of the frame is discarded. If the application requests more bytes than are in the current frame, only the current frame's data is returned.
This patch utilizes common code derived from RFCOMM's recvmsg() function to make L2CAP SOCK_STREAM reads behave like RFCOMM reads (and other SOCK_STREAM sockets in general). The application may read one byte at a time from the input stream and not lose any data, and may also read across L2CAP frame boundaries.
--- net/bluetooth/l2cap.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
net/bluetooth/l2cap.c +3 -0
@@ -1959,6 +1959,9 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct msrelease_sock (sk); +if (sock->type ==SOCK_STREAM ) +return bt_sock_stream_recvmsg (iocb, sock, msg, len, flags); +return bt_sock_recvmsg (iocb, sock, msg, len, flags); }
Signature · 6 lines
1.7.1 -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum