Hi Marcel,
On Tue, Aug 31, 2021 at 1:35 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> > bt_skb_sendmsg helps takes care of allocation the skb and copying the
> > the contents of msg over to the skb while checking for possible errors
> > so it should be safe to call it without holding lock_sock.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > ---
> > include/net/bluetooth/bluetooth.h | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 9125effbf448..1bfb5d3d24dd 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -420,6 +420,29 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk,
> > return NULL;
> > }
> >
> > +/* Shall not be called with lock_sock held */
> > +static inline struct sk_buff *bt_skb_sendmsg(struct sock *sk,
> > + struct msghdr *msg,
> > + size_t len, int reserve)
> > +{
> > + struct sk_buff *skb;
> > + int err;
> > +
> > + skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err);
> > + if (!skb)
> > + return ERR_PTR(err);
> > +
> > + if (memcpy_from_msg(skb_put(skb, len), msg, len)) {
> > + kfree_skb(skb);
> > + return ERR_PTR(-EFAULT);
> > + }
> > +
> > + skb_reserve(skb, reserve);
> > + skb->priority = sk->sk_priority;
>
> have you tested this? I remember vaguely (really vaguely actually) that skb_reserve needs to be called before you do any changes to the buffer via skb_put and others. Especially since bt_skb_send_alloc already calls skb_reserve in the first place.
Yep, that and the fact that we need both a header and footer size to
be given separately since the len should only account for the payload
itself, anyway I will send an update shortly and Ive now tested with
rfcomm-tester and they are all passing.
> Regards
>
> Marcel
>
--
Luiz Augusto von Dentz
