Hi Jukka,
> 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.
is this really the best approach. What are other subsystems doing?
> Signed-off-by: Jukka Rissanen <jukka.rissanen@xxxxxxxxxxxxxxx>
> ---
> net/bluetooth/l2cap_core.c | 25 +++++++++++++++++++++----
> net/bluetooth/l2cap_sock.c | 25 ++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index a1e5bb7..528b38a 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -2098,6 +2098,24 @@ static void l2cap_send_ack(struct l2cap_chan *chan)
> }
> }
>
> +static inline void memcpy_fromiovec_kernel(void *kdata, struct iovec *iov,
> + int len)
> +{
> + while (len > 0) {
> + if (iov->iov_len) {
> + int copy = min_t(unsigned int, len, iov->iov_len);
> +
> + memcpy(kdata, iov->iov_base, copy);
> +
> + len -= copy;
> + kdata += copy;
> + iov->iov_base += copy;
> + iov->iov_len -= copy;
> + }
> + iov++;
> + }
> +}
I am a bit amazed that there are no existing helper functions for this. Should we add this to lib/iovec.c? At least lets check what the upstream guys think.
> +
> static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
> struct msghdr *msg, int len,
> int count, struct sk_buff *skb)
> @@ -2106,8 +2124,7 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
> struct sk_buff **frag;
> int sent = 0;
>
> - if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
> - return -EFAULT;
> + memcpy_fromiovec_kernel(skb_put(skb, count), msg->msg_iov, count);
>
> sent += count;
> len -= count;
> @@ -2126,8 +2143,8 @@ static inline int l2cap_skbuff_fromiovec(struct l2cap_chan *chan,
>
> *frag = tmp;
>
> - if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
> - return -EFAULT;
> + memcpy_fromiovec_kernel(skb_put(*frag, count), msg->msg_iov,
> + count);
>
> (*frag)->priority = skb->priority;
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index f59e00c..bbee8dc 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -948,6 +948,9 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
> {
> struct sock *sk = sock->sk;
> struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> + struct msghdr kernel_msg;
> + struct kvec iv;
> + void *buf;
> int err;
>
> BT_DBG("sock %p, sk %p", sock, sk);
> @@ -968,10 +971,30 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
> if (err)
> return err;
>
> + buf = kmalloc(len, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + if (memcpy_fromiovec(buf, msg->msg_iov, len)) {
> + err = -EFAULT;
> + goto done;
> + }
> +
> + iv.iov_base = buf;
> + iv.iov_len = len;
> +
> + memset(&kernel_msg, 0, sizeof(kernel_msg));
> +
> + kernel_msg.msg_iov = (struct iovec *) &iv;
> + kernel_msg.msg_iovlen = 1;
> + kernel_msg.msg_flags = msg->msg_flags;
> +
> l2cap_chan_lock(chan);
> - err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
> + err = l2cap_chan_send(chan, &kernel_msg, len, sk->sk_priority);
If we move this here. Why are we keeping input variable as an IOV? Will the kernel internal code ever use or require an IOV. We could just make l2cap_chan_send flat with a single buffer.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
