Return-Path: <cyrus@holtmann.org>
Content-Type: text/plain; charset=windows-1252
Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.2\))
Subject: Re: [PATCH v2 1/8] Bluetooth: Refactor l2cap_sock_sendmsg() to copy user buffer
From: Marcel Holtmann <marcel@holtmann.org>
In-Reply-To: <1401092222.2400.18.camel@jrissane-mobl.ger.corp.intel.com>
Date: Mon, 26 May 2014 14:13:08 +0200
Cc: linux-bluetooth@vger.kernel.org
Message-Id: <09DFE249-71A0-44E6-8552-0BDE5CC656EF@holtmann.org>
References: <1400837248-12179-1-git-send-email-jukka.rissanen@linux.intel.com> <1400837248-12179-2-git-send-email-jukka.rissanen@linux.intel.com> <98B7DE79-FFC6-435F-86DA-36F11C8F7C2B@holtmann.org> <1401092222.2400.18.camel@jrissane-mobl.ger.corp.intel.com>
To: Jukka Rissanen <jukka.rissanen@linux.intel.com>
Sender: linux-bluetooth-owner@vger.kernel.org
List-ID: <linux-bluetooth.vger.kernel.org>

Hi Jukka,

>>> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
>>> index f59e00c..7e7b28a 100644
>>> --- a/net/bluetooth/l2cap_sock.c
>>> +++ b/net/bluetooth/l2cap_sock.c
>>> @@ -948,6 +948,7 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
>>> {
>>> 	struct sock *sk = sock->sk;
>>> 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
>>> +	unsigned char *buf;
>>> 	int err;
>>> 
>>> 	BT_DBG("sock %p, sk %p", sock, sk);
>>> @@ -968,10 +969,21 @@ 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;
>>> +	}
>>> +
>>> 	l2cap_chan_lock(chan);
>>> -	err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
>>> +	err = l2cap_chan_send(chan, buf, len, sk->sk_priority, msg->msg_flags);
>>> 	l2cap_chan_unlock(chan);
>>> 
>>> +done:
>>> +	kfree(buf);
>>> 	return err;
>>> }
>> 
>> We don?t have to use chan->ops->alloc_skb here? Has this become obsolete now?
> 
> Isn't the buf here just a normal memory buffer that stores just the user
> data and has nothing to do with sk_buff that is returned by alloc_skb()?

I asked the question, because I do not know. Do we need alloc_skb callback at all.

Regards

Marcel