Re: [RFC 1/4] Bluetooth: Add bt_skb_sendmsg helper

Hi Marcel,

On Tue, Aug 31, 2021 at 1:35 AM Marcel Holtmann [off-list ref] wrote:

Hi Luiz,

quoted

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 <redacted>
---
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