2010-09-08 17:05:25

by Mat Martineau

[permalink] [raw]
Subject: [PATCH 0/4 v6] L2CAP updates for valid PSMs, SOCK_STREAM reads

This patch set is the same as 'v5', with a fix to PSM validation.


[PATCH 1/4] Bluetooth: Validate PSM values in calls to connect() and bind()

Modified to bypass PSM validation for SOCK_RAW (bonding) sockets.


[PATCH 2/4] Bluetooth: Add common code for stream-oriented recvmsg()
[PATCH 3/4] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM
[PATCH 4/4] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.

These patches change the read behavior of SOCK_STREAM L2CAP sockets to
allow reading of partial SDUs. Same as the 'v5' patches.


2010-09-27 20:38:31

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 0/4 v6] L2CAP updates for valid PSMs, SOCK_STREAM reads

Hi Mat,

* Mat Martineau <[email protected]> [2010-09-08 10:05:25 -0700]:

> This patch set is the same as 'v5', with a fix to PSM validation.
>
>
> [PATCH 1/4] Bluetooth: Validate PSM values in calls to connect() and bind()
>
> Modified to bypass PSM validation for SOCK_RAW (bonding) sockets.
>
>
> [PATCH 2/4] Bluetooth: Add common code for stream-oriented recvmsg()
> [PATCH 3/4] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM
> [PATCH 4/4] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
>
> These patches change the read behavior of SOCK_STREAM L2CAP sockets to
> allow reading of partial SDUs. Same as the 'v5' patches.

All 4 patches have been applied to my bluetoot-next-2.6 tree for later
merge in wireless-next-2.6

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-09-20 21:14:47

by Ron Shaffer

[permalink] [raw]
Subject: Re: [PATCH 0/4 v6] L2CAP updates for valid PSMs, SOCK_STREAM reads

On 9/8/2010 12:05 PM, Mat Martineau wrote:
> This patch set is the same as 'v5', with a fix to PSM validation.
>
>
> [PATCH 1/4] Bluetooth: Validate PSM values in calls to connect() and bind()
>
> Modified to bypass PSM validation for SOCK_RAW (bonding) sockets.
>
>
> [PATCH 2/4] Bluetooth: Add common code for stream-oriented recvmsg()
> [PATCH 3/4] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM
> [PATCH 4/4] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.
>
> These patches change the read behavior of SOCK_STREAM L2CAP sockets to
> allow reading of partial SDUs. Same as the 'v5' patches.
>

Ping

--
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2010-09-08 19:47:45

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 4/4] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.

* Mat Martineau <[email protected]> [2010-09-08 10:05:29 -0700]:

> L2CAP 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.
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> net/bluetooth/l2cap.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Gustavo F. Padovan <[email protected]>

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-09-08 19:47:09

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 3/4] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM

* Mat Martineau <[email protected]> [2010-09-08 10:05:28 -0700]:

> To 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().
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> net/bluetooth/rfcomm/sock.c | 104 +++----------------------------------------
> 1 files changed, 6 insertions(+), 98 deletions(-)

Acked-by: Gustavo F. Padovan <[email protected]>

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-09-08 19:46:25

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 2/4] Bluetooth: Add common code for stream-oriented recvmsg()

* Mat Martineau <[email protected]> [2010-09-08 10:05:27 -0700]:

> This 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().
>
> Signed-off-by: Mat Martineau <[email protected]>
> ---
> include/net/bluetooth/bluetooth.h | 2 +
> net/bluetooth/af_bluetooth.c | 109 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 111 insertions(+), 0 deletions(-)

Acked-by: Gustavo F. Padovan <[email protected]>

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-09-08 19:45:40

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 1/4] Bluetooth: Validate PSM values in calls to connect() and bind()

* Mat Martineau <[email protected]> [2010-09-08 10:05:26 -0700]:

> 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 | 25 +++++++++++++++++++++----
> 1 files changed, 21 insertions(+), 4 deletions(-)

Acked-by: Gustavo F. Padovan <[email protected]>

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-09-08 17:05:29

by Mat Martineau

[permalink] [raw]
Subject: [PATCH 4/4] Bluetooth: Use a stream-oriented recvmsg with SOCK_STREAM L2CAP sockets.

L2CAP 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.

Signed-off-by: Mat Martineau <[email protected]>
---
net/bluetooth/l2cap.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index b5eff42..38aa78c 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1961,6 +1961,9 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms

release_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);
}

--
1.7.1

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2010-09-08 17:05:27

by Mat Martineau

[permalink] [raw]
Subject: [PATCH 2/4] Bluetooth: Add common code for stream-oriented recvmsg()

This 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().

Signed-off-by: Mat Martineau <[email protected]>
---
include/net/bluetooth/bluetooth.h | 2 +
net/bluetooth/af_bluetooth.c | 109 +++++++++++++++++++++++++++++++++++++
2 files changed, 111 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..08b6c2a 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -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);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 421c45b..77a26fe 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -265,6 +265,115 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
}
EXPORT_SYMBOL(bt_sock_recvmsg);

+static long bt_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))
+ break;
+
+ if (sk->sk_err || (sk->sk_shutdown & RCV_SHUTDOWN))
+ break;
+
+ if (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;
+}
+
+int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *msg, size_t size, int flags)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ size_t target, copied = 0;
+ long timeo;
+
+ if (flags & MSG_OOB)
+ return -EOPNOTSUPP;
+
+ msg->msg_namelen = 0;
+
+ BT_DBG("sk %p size %zu", sk, size);
+
+ lock_sock(sk);
+
+ 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 = bt_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)) {
+ 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:
+ release_sock(sk);
+ return copied ? : err;
+}
+EXPORT_SYMBOL(bt_sock_stream_recvmsg);
+
static inline unsigned int bt_accept_poll(struct sock *parent)
{
struct list_head *p, *n;
--
1.7.1

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2010-09-08 17:05:28

by Mat Martineau

[permalink] [raw]
Subject: [PATCH 3/4] Bluetooth: Use common SOCK_STREAM receive code in RFCOMM

To 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().

Signed-off-by: Mat Martineau <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 104 +++----------------------------------------
1 files changed, 6 insertions(+), 98 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 44a6232..4396f47 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -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)
--
1.7.1

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

2010-09-08 17:05:26

by Mat Martineau

[permalink] [raw]
Subject: [PATCH 1/4] Bluetooth: Validate PSM values in calls to connect() and bind()

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 | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c784703..b5eff42 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1008,10 +1008,20 @@ 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 (la.l2_psm) {
+ __u16 psm = __le16_to_cpu(la.l2_psm);
+
+ /* PSM must be odd and lsb of upper byte must be 0 */
+ if ((psm & 0x0101) != 0x0001) {
+ err = -EINVAL;
+ goto done;
+ }
+
+ /* Restrict usage of well-known PSMs */
+ if (psm < 0x1001 && !capable(CAP_NET_BIND_SERVICE)) {
+ err = -EACCES;
+ goto done;
+ }
}

write_lock_bh(&l2cap_sk_list.lock);
@@ -1190,6 +1200,13 @@ 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 &&
+ sk->sk_type != SOCK_RAW) {
+ err = -EINVAL;
+ goto done;
+ }
+
/* Set destination address and psm */
bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr);
l2cap_pi(sk)->psm = la.l2_psm;
--
1.7.1

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum