2014-06-06 13:15:42

by Jukka Rissanen

[permalink] [raw]
Subject: [RFC v2] Bluetooth: Refactor l2cap_sock_sendmsg() to copy user buffer

The l2cap_chan_send() is changed to use kernel memory directly,
so this function must read the user buffer before sending the
message.

The change is done as the 6LoWPAN also uses l2cap_chan_send()
and in order to minimize the amount of code changes, we must
copy the user buffer in sock handling code.

Signed-off-by: Jukka Rissanen <[email protected]>
---
Hi,

this is take 2 of the refactoring l2cap_sock_sendmsg(). Instead of
passing func pointer like in v1, I added callbacks to l2cap_ops struct
that handle the skb delivery.

The code is not really tested or ready yet but compiles ok.
Any opinions of this solution?


Cheers,
Jukka



include/net/bluetooth/l2cap.h | 27 ++++++-
net/bluetooth/a2mp.c | 12 +---
net/bluetooth/l2cap_core.c | 160 ++++++++++++++++++++++++++++++++++--------
net/bluetooth/l2cap_sock.c | 3 +-
4 files changed, 157 insertions(+), 45 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 4abdcb2..d61f8c8 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -601,6 +601,17 @@ struct l2cap_ops {
long (*get_sndtimeo) (struct l2cap_chan *chan);
struct sk_buff *(*alloc_skb) (struct l2cap_chan *chan,
unsigned long len, int nb);
+ int (*user_delivery) (struct l2cap_chan *chan,
+ const struct msghdr *msg,
+ int len, unsigned int flags,
+ int count,
+ struct sk_buff *skb);
+ int (*kernel_delivery) (struct l2cap_chan *chan,
+ const void *buf,
+ int len,
+ unsigned int flags,
+ int count,
+ struct sk_buff *skb);
};

struct l2cap_conn {
@@ -872,8 +883,20 @@ struct l2cap_chan *l2cap_chan_create(void);
void l2cap_chan_close(struct l2cap_chan *chan, int reason);
int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
bdaddr_t *dst, u8 dst_type);
-int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
- u32 priority);
+
+int l2cap_chan_send(struct l2cap_chan *chan, const void *buf,
+ size_t len, u32 priority, unsigned int flags);
+int l2cap_chan_send_fromiovec(struct l2cap_chan *chan,
+ const struct msghdr *msg, size_t len,
+ u32 priority);
+int l2cap_default_user_deliver_cb(struct l2cap_chan *chan,
+ const struct msghdr *msg, int len,
+ unsigned int flags, int count,
+ struct sk_buff *skb);
+int l2cap_default_kernel_deliver_cb(struct l2cap_chan *chan,
+ const void *buf, int len,
+ unsigned int flags, int count,
+ struct sk_buff *skb);
void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
int l2cap_chan_check_security(struct l2cap_chan *chan);
void l2cap_chan_set_defaults(struct l2cap_chan *chan);
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index 9514cc9..6b99b1b 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -48,22 +48,12 @@ void a2mp_send(struct amp_mgr *mgr, u8 code, u8 ident, u16 len, void *data)
struct l2cap_chan *chan = mgr->a2mp_chan;
struct a2mp_cmd *cmd;
u16 total_len = len + sizeof(*cmd);
- struct kvec iv;
- struct msghdr msg;

cmd = __a2mp_build(code, ident, len, data);
if (!cmd)
return;

- iv.iov_base = cmd;
- iv.iov_len = total_len;
-
- memset(&msg, 0, sizeof(msg));
-
- msg.msg_iov = (struct iovec *) &iv;
- msg.msg_iovlen = 1;
-
- l2cap_chan_send(chan, &msg, total_len, 0);
+ l2cap_chan_send(chan, cmd, total_len, 0, 0);

kfree(cmd);
}
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7468482..8e95858 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2105,7 +2105,8 @@ static void l2cap_send_ack(struct l2cap_chan *chan)
}

static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
- struct msghdr *msg, int len,
+ const struct msghdr *msg, int len,
+ unsigned int flags,
int count, struct sk_buff *skb)
{
struct l2cap_conn *conn = chan->conn;
@@ -2126,13 +2127,14 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
count = min_t(unsigned int, conn->mtu, len);

tmp = chan->ops->alloc_skb(chan, count,
- msg->msg_flags & MSG_DONTWAIT);
+ flags & MSG_DONTWAIT);
if (IS_ERR(tmp))
return PTR_ERR(tmp);

*frag = tmp;

- if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
+ if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov,
+ count))
return -EFAULT;

(*frag)->priority = skb->priority;
@@ -2149,9 +2151,90 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
return sent;
}

+int l2cap_default_user_deliver_cb(struct l2cap_chan *chan,
+ const struct msghdr *msg, int len,
+ unsigned int flags, int count,
+ struct sk_buff *skb)
+{
+ return l2cap_skbuff_fromiovec(chan, msg, len, flags, count, skb);
+}
+EXPORT_SYMBOL_GPL(l2cap_default_user_deliver_cb);
+
+static inline int l2cap_copy_into_skbuff(struct l2cap_chan *chan,
+ const void *buf, int len,
+ unsigned int flags, int count,
+ struct sk_buff *skb)
+{
+ struct l2cap_conn *conn = chan->conn;
+ struct sk_buff **frag;
+ int sent = 0;
+
+ memcpy(skb_put(skb, count), buf, count);
+
+ sent += count;
+ len -= count;
+ buf += count;
+
+ /* Continuation fragments (no L2CAP header) */
+ frag = &skb_shinfo(skb)->frag_list;
+ while (len) {
+ struct sk_buff *tmp;
+
+ count = min_t(unsigned int, conn->mtu, len);
+
+ tmp = chan->ops->alloc_skb(chan, count,
+ flags & MSG_DONTWAIT);
+ if (IS_ERR(tmp))
+ return PTR_ERR(tmp);
+
+ *frag = tmp;
+
+ memcpy(skb_put(*frag, count), buf, count);
+
+ (*frag)->priority = skb->priority;
+
+ sent += count;
+ len -= count;
+ buf += count;
+
+ skb->len += (*frag)->len;
+ skb->data_len += (*frag)->len;
+
+ frag = &(*frag)->next;
+ }
+
+ return sent;
+}
+
+int l2cap_default_kernel_deliver_cb(struct l2cap_chan *chan,
+ const void *buf, int len,
+ unsigned int flags, int count,
+ struct sk_buff *skb)
+{
+ return l2cap_copy_into_skbuff(chan, buf, len, flags, count, skb);
+}
+EXPORT_SYMBOL_GPL(l2cap_default_kernel_deliver_cb);
+
+int l2cap_deliver_skb(struct l2cap_chan *chan,
+ const void *buf, int len,
+ unsigned int flags, int count,
+ struct sk_buff *skb)
+{
+ if (chan->ops->user_delivery)
+ return chan->ops->user_delivery(chan, buf, len, flags, count,
+ skb);
+
+ if (chan->ops->kernel_delivery)
+ return chan->ops->kernel_delivery(chan, buf, len, flags, count,
+ skb);
+
+ return -EINVAL;
+}
+
static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
- struct msghdr *msg, size_t len,
- u32 priority)
+ const void *buf, size_t len,
+ u32 priority,
+ unsigned int flags)
{
struct l2cap_conn *conn = chan->conn;
struct sk_buff *skb;
@@ -2164,7 +2247,7 @@ static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
count = min_t(unsigned int, (conn->mtu - hlen), len);

skb = chan->ops->alloc_skb(chan, count + hlen,
- msg->msg_flags & MSG_DONTWAIT);
+ flags & MSG_DONTWAIT);
if (IS_ERR(skb))
return skb;

@@ -2176,7 +2259,7 @@ static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
lh->len = cpu_to_le16(len + L2CAP_PSMLEN_SIZE);
put_unaligned(chan->psm, (__le16 *) skb_put(skb, L2CAP_PSMLEN_SIZE));

- err = l2cap_skbuff_fromiovec(chan, msg, len, count, skb);
+ err = l2cap_deliver_skb(chan, buf, len, flags, count, skb);
if (unlikely(err < 0)) {
kfree_skb(skb);
return ERR_PTR(err);
@@ -2185,8 +2268,8 @@ static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
}

static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
- struct msghdr *msg, size_t len,
- u32 priority)
+ const void *buf, size_t len,
+ u32 priority, unsigned int flags)
{
struct l2cap_conn *conn = chan->conn;
struct sk_buff *skb;
@@ -2198,7 +2281,7 @@ static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
count = min_t(unsigned int, (conn->mtu - L2CAP_HDR_SIZE), len);

skb = chan->ops->alloc_skb(chan, count + L2CAP_HDR_SIZE,
- msg->msg_flags & MSG_DONTWAIT);
+ flags & MSG_DONTWAIT);
if (IS_ERR(skb))
return skb;

@@ -2209,7 +2292,7 @@ static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
lh->cid = cpu_to_le16(chan->dcid);
lh->len = cpu_to_le16(len);

- err = l2cap_skbuff_fromiovec(chan, msg, len, count, skb);
+ err = l2cap_deliver_skb(chan, buf, len, flags, count, skb);
if (unlikely(err < 0)) {
kfree_skb(skb);
return ERR_PTR(err);
@@ -2218,8 +2301,8 @@ static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
}

static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
- struct msghdr *msg, size_t len,
- u16 sdulen)
+ const void *buf, size_t len,
+ u16 sdulen, unsigned int flags)
{
struct l2cap_conn *conn = chan->conn;
struct sk_buff *skb;
@@ -2242,7 +2325,7 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
count = min_t(unsigned int, (conn->mtu - hlen), len);

skb = chan->ops->alloc_skb(chan, count + hlen,
- msg->msg_flags & MSG_DONTWAIT);
+ flags & MSG_DONTWAIT);
if (IS_ERR(skb))
return skb;

@@ -2260,7 +2343,7 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
if (sdulen)
put_unaligned_le16(sdulen, skb_put(skb, L2CAP_SDULEN_SIZE));

- err = l2cap_skbuff_fromiovec(chan, msg, len, count, skb);
+ err = l2cap_deliver_skb(chan, buf, len, flags, count, skb);
if (unlikely(err < 0)) {
kfree_skb(skb);
return ERR_PTR(err);
@@ -2273,14 +2356,15 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,

static int l2cap_segment_sdu(struct l2cap_chan *chan,
struct sk_buff_head *seg_queue,
- struct msghdr *msg, size_t len)
+ const void *buf, size_t len,
+ unsigned int flags)
{
struct sk_buff *skb;
u16 sdu_len;
size_t pdu_len;
u8 sar;

- BT_DBG("chan %p, msg %p, len %zu", chan, msg, len);
+ BT_DBG("chan %p, buf %p, len %zu", chan, buf, len);

/* It is critical that ERTM PDUs fit in a single HCI fragment,
* so fragmented skbs are not used. The HCI layer's handling
@@ -2314,7 +2398,8 @@ static int l2cap_segment_sdu(struct l2cap_chan *chan,
}

while (len > 0) {
- skb = l2cap_create_iframe_pdu(chan, msg, pdu_len, sdu_len);
+ skb = l2cap_create_iframe_pdu(chan, buf, pdu_len, sdu_len,
+ flags);

if (IS_ERR(skb)) {
__skb_queue_purge(seg_queue);
@@ -2342,8 +2427,9 @@ static int l2cap_segment_sdu(struct l2cap_chan *chan,
}

static struct sk_buff *l2cap_create_le_flowctl_pdu(struct l2cap_chan *chan,
- struct msghdr *msg,
- size_t len, u16 sdulen)
+ const void *buf, size_t len,
+ u16 sdulen,
+ unsigned int flags)
{
struct l2cap_conn *conn = chan->conn;
struct sk_buff *skb;
@@ -2363,7 +2449,7 @@ static struct sk_buff *l2cap_create_le_flowctl_pdu(struct l2cap_chan *chan,
count = min_t(unsigned int, (conn->mtu - hlen), len);

skb = chan->ops->alloc_skb(chan, count + hlen,
- msg->msg_flags & MSG_DONTWAIT);
+ flags & MSG_DONTWAIT);
if (IS_ERR(skb))
return skb;

@@ -2375,7 +2461,7 @@ static struct sk_buff *l2cap_create_le_flowctl_pdu(struct l2cap_chan *chan,
if (sdulen)
put_unaligned_le16(sdulen, skb_put(skb, L2CAP_SDULEN_SIZE));

- err = l2cap_skbuff_fromiovec(chan, msg, len, count, skb);
+ err = l2cap_deliver_skb(chan, buf, len, flags, count, skb);
if (unlikely(err < 0)) {
kfree_skb(skb);
return ERR_PTR(err);
@@ -2386,13 +2472,14 @@ static struct sk_buff *l2cap_create_le_flowctl_pdu(struct l2cap_chan *chan,

static int l2cap_segment_le_sdu(struct l2cap_chan *chan,
struct sk_buff_head *seg_queue,
- struct msghdr *msg, size_t len)
+ const void *buf, size_t len,
+ unsigned int flags)
{
struct sk_buff *skb;
size_t pdu_len;
u16 sdu_len;

- BT_DBG("chan %p, msg %p, len %zu", chan, msg, len);
+ BT_DBG("chan %p, buf %p, len %zu", chan, buf, len);

pdu_len = chan->conn->mtu - L2CAP_HDR_SIZE;

@@ -2405,7 +2492,8 @@ static int l2cap_segment_le_sdu(struct l2cap_chan *chan,
if (len <= pdu_len)
pdu_len = len;

- skb = l2cap_create_le_flowctl_pdu(chan, msg, pdu_len, sdu_len);
+ skb = l2cap_create_le_flowctl_pdu(chan, buf, pdu_len, sdu_len,
+ flags);
if (IS_ERR(skb)) {
__skb_queue_purge(seg_queue);
return PTR_ERR(skb);
@@ -2414,6 +2502,7 @@ static int l2cap_segment_le_sdu(struct l2cap_chan *chan,
__skb_queue_tail(seg_queue, skb);

len -= pdu_len;
+ buf += pdu_len;

if (sdu_len) {
sdu_len = 0;
@@ -2424,8 +2513,8 @@ static int l2cap_segment_le_sdu(struct l2cap_chan *chan,
return 0;
}

-int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
- u32 priority)
+int l2cap_chan_send(struct l2cap_chan *chan, const void *buf, size_t len,
+ u32 priority, unsigned int flags)
{
struct sk_buff *skb;
int err;
@@ -2436,7 +2525,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,

/* Connectionless channel */
if (chan->chan_type == L2CAP_CHAN_CONN_LESS) {
- skb = l2cap_create_connless_pdu(chan, msg, len, priority);
+ skb = l2cap_create_connless_pdu(chan, buf, len, priority,
+ flags);
if (IS_ERR(skb))
return PTR_ERR(skb);

@@ -2463,7 +2553,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,

__skb_queue_head_init(&seg_queue);

- err = l2cap_segment_le_sdu(chan, &seg_queue, msg, len);
+ err = l2cap_segment_le_sdu(chan, &seg_queue, buf, len, flags);

if (chan->state != BT_CONNECTED) {
__skb_queue_purge(&seg_queue);
@@ -2493,7 +2583,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
return -EMSGSIZE;

/* Create a basic PDU */
- skb = l2cap_create_basic_pdu(chan, msg, len, priority);
+ skb = l2cap_create_basic_pdu(chan, buf, len, priority, flags);
if (IS_ERR(skb))
return PTR_ERR(skb);

@@ -2523,7 +2613,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
* since it's possible to block while waiting for memory
* allocation.
*/
- err = l2cap_segment_sdu(chan, &seg_queue, msg, len);
+ err = l2cap_segment_sdu(chan, &seg_queue, buf, len, flags);

/* The channel could have been closed while segmenting,
* check that it is still connected.
@@ -2557,6 +2647,14 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
return err;
}

+int l2cap_chan_send_fromiovec(struct l2cap_chan *chan,
+ const struct msghdr *msg, size_t len,
+ u32 priority)
+{
+ return l2cap_chan_send(chan, msg, len, priority, msg->msg_flags);
+}
+EXPORT_SYMBOL_GPL(l2cap_chan_send_fromiovec);
+
static void l2cap_send_srej(struct l2cap_chan *chan, u16 txseq)
{
struct l2cap_ctrl control;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index f59e00c..315424e 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -969,7 +969,7 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
return err;

l2cap_chan_lock(chan);
- err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
+ err = l2cap_chan_send_fromiovec(chan, msg, len, sk->sk_priority);
l2cap_chan_unlock(chan);

return err;
@@ -1391,6 +1391,7 @@ static struct l2cap_ops l2cap_chan_ops = {
.set_shutdown = l2cap_sock_set_shutdown_cb,
.get_sndtimeo = l2cap_sock_get_sndtimeo_cb,
.alloc_skb = l2cap_sock_alloc_skb_cb,
+ .user_delivery = l2cap_default_user_deliver_cb,
};

static void l2cap_sock_destruct(struct sock *sk)
--
1.8.3.1