Hey Luiz.
Sorry for keeping you waiting. I will send it today.

pon., 31 sty 2022 o 19:47 Luiz Augusto von Dentz
[off-list ref] napisał(a):

Hi Radosław,

On Thu, Jan 13, 2022 at 2:23 PM Luiz Augusto von Dentz
[off-list ref] wrote:

quoted

Hi Radosław,

On Thu, Jan 13, 2022 at 2:07 PM Radosław Biernacki [off-list ref] wrote:

quoted

Hi Luiz,

czw., 13 sty 2022 o 17:17 Luiz Augusto von Dentz
[off-list ref] napisał(a):

quoted

Hi Radoslaw,

On Thu, Jan 13, 2022 at 7:09 AM Radoslaw Biernacki [off-list ref] wrote:

quoted

From: Radoslaw Biernacki <redacted>

This patch fixes skb allocation, as lack of space for ev might push skb
tail beyond its end.
Also introduce eir_precalc_len() that can be used instead of magic
numbers for similar eir operations on skb.

Fixes: cf1bce1de7eeb ("Bluetooth: mgmt: Make use of mgmt_send_event_skb in MGMT_EV_DEVICE_FOUND")
Signed-off-by: Angela Czubak <redacted>
Signed-off-by: Marek Maslanka <redacted>
Signed-off-by: Radoslaw Biernacki <redacted>
---
 net/bluetooth/eir.h  |  5 +++++
 net/bluetooth/mgmt.c | 12 ++++--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/eir.h b/net/bluetooth/eir.h
index 05e2e917fc25..e5876751f07e 100644
--- a/net/bluetooth/eir.h
+++ b/net/bluetooth/eir.h

@@ -15,6 +15,11 @@ u8 eir_create_scan_rsp(struct hci_dev *hdev, u8 instance, u8 *ptr);
 u8 eir_append_local_name(struct hci_dev *hdev, u8 *eir, u8 ad_len);
 u8 eir_append_appearance(struct hci_dev *hdev, u8 *ptr, u8 ad_len);

+static inline u16 eir_precalc_len(u8 data_len)
+{
+       return sizeof(u8) * 2 + data_len;
+}
+
 static inline u16 eir_append_data(u8 *eir, u16 eir_len, u8 type,
                                  u8 *data, u8 data_len)
 {

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 37087cf7dc5a..d517fd847730 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c

@@ -9680,13 +9680,11 @@ void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
 {
        struct sk_buff *skb;
        struct mgmt_ev_device_found *ev;
-       u16 eir_len;
-       u32 flags;
+       u16 eir_len = 0;
+       u32 flags = 0;

-       if (name_len)
-               skb = mgmt_alloc_skb(hdev, MGMT_EV_DEVICE_FOUND, 2 + name_len);
-       else
-               skb = mgmt_alloc_skb(hdev, MGMT_EV_DEVICE_FOUND, 0);
+       skb = mgmt_alloc_skb(hdev, MGMT_EV_DEVICE_FOUND,
+                            sizeof(*ev) + (name ? eir_precalc_len(name_len) : 0));

Looks like mgmt_device_connected also has a similar problem.

Yes, I was planning to send a patch to this one though it will not be as slick.
It would be nice to have a helper which will call skb_put() and add
eir data at once.
Basically skb operation in pair to, what eir_append_data() does with
help of eir_len but without awkwardness when passing return value to
skb_put() (as it returns offset not size).

Hmm, that might be a good idea indeed something like eir_append_skb,
if only we could grow the skb with skb_put directly that would
eliminate the problem with having to reserve enough space for the
worse case.

quoted

I will send V2 with two patches. I hope they will align with your
original goal of eliminating the necessity of intermediary buffers at
some point in future.

Are you still planning to send the v2?

quoted

quoted

quoted

quoted

        ev = skb_put(skb, sizeof(*ev));
        bacpy(&ev->addr.bdaddr, bdaddr);

@@ -9696,10 +9694,8 @@ void mgmt_remote_name(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
        if (name) {
                eir_len = eir_append_data(ev->eir, 0, EIR_NAME_COMPLETE, name,
                                          name_len);
-               flags = 0;
                skb_put(skb, eir_len);
        } else {
-               eir_len = 0;
                flags = MGMT_DEV_FOUND_NAME_REQUEST_FAILED;
        }

These changes would leave flags and eir_len uninitialized.

Both are initialized to 0 by this patch.

Sorry, I must be blind that I didn't see you had changed that to be
initialized in their declaration.

quoted

quoted

quoted

--
2.34.1.703.g22d0c6ccf7-goog

--
Luiz Augusto von Dentz

--
Luiz Augusto von Dentz

--
Luiz Augusto von Dentz