2022-06-06 05:30:36

by Yury Norov

[permalink] [raw]
Subject: [PATCH] net/bluetooth: fix erroneous use of bitmap_from_u64()

The commit 0a97953fd221 ("lib: add bitmap_{from,to}_arr64") changed
implementation of bitmap_from_u64(), so that it doesn't typecast
argument to u64, and actually dereferences memory.

With that change, compiler spotted few places in bluetooth code
where bitmap_from_u64 is called for 32-bit variable.

As reported by Sudip Mukherjee:

"arm allmodconfig" fails with the error:

In file included from ./include/linux/string.h:253,
from ./include/linux/bitmap.h:11,
from ./include/linux/cpumask.h:12,
from ./include/linux/smp.h:13,
from ./include/linux/lockdep.h:14,
from ./include/linux/mutex.h:17,
from ./include/linux/rfkill.h:35,
from net/bluetooth/hci_core.c:29:
In function 'fortify_memcpy_chk',
inlined from 'bitmap_copy' at ./include/linux/bitmap.h:254:2,
inlined from 'bitmap_copy_clear_tail' at ./include/linux/bitmap.h:263:2,
inlined from 'bitmap_from_u64' at ./include/linux/bitmap.h:540:2,
inlined from 'hci_bdaddr_list_add_with_flags' at net/bluetooth/hci_core.c:2156:2:
./include/linux/fortify-string.h:344:25: error: call to '__write_overflow_field' declared with attribute warning:
+detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
344 | __write_overflow_field(p_size_field, size);

And, "csky allmodconfig" fails with the error:

In file included from ./include/linux/cpumask.h:12,
from ./include/linux/mm_types_task.h:14,
from ./include/linux/mm_types.h:5,
from ./include/linux/buildid.h:5,
from ./include/linux/module.h:14,
from net/bluetooth/mgmt.c:27:
In function 'bitmap_copy',
inlined from 'bitmap_copy_clear_tail' at ./include/linux/bitmap.h:263:2,
inlined from 'bitmap_from_u64' at ./include/linux/bitmap.h:540:2,
inlined from 'set_device_flags' at net/bluetooth/mgmt.c:4534:4:
./include/linux/bitmap.h:254:9: error: 'memcpy' forming offset [4, 7] is out of the bounds [0, 4] of object 'flags'
+with type 'long unsigned int[1]' [-Werror=array-bounds]
254 | memcpy(dst, src, len);
| ^~~~~~~~~~~~~~~~~~~~~
In file included from ./include/linux/kasan-checks.h:5,
from ./include/asm-generic/rwonce.h:26,
from ./arch/csky/include/generated/asm/rwonce.h:1,
from ./include/linux/compiler.h:248,
from ./include/linux/build_bug.h:5,
from ./include/linux/container_of.h:5,
from ./include/linux/list.h:5,
from ./include/linux/module.h:12,
from net/bluetooth/mgmt.c:27:
net/bluetooth/mgmt.c: In function 'set_device_flags':
net/bluetooth/mgmt.c:4532:40: note: 'flags' declared here
4532 | DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);
| ^~~~~
./include/linux/types.h:11:23: note: in definition of macro 'DECLARE_BITMAP'
11 | unsigned long name[BITS_TO_LONGS(bits)]

Fix it by replacing bitmap_from_u64 with bitmap_from_arr32.

Reported-by: Sudip Mukherjee <[email protected]com>
Signed-off-by: Yury Norov <[email protected]>
---
net/bluetooth/hci_core.c | 2 +-
net/bluetooth/mgmt.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5abb2ca5b129..2de7e1ec4035 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2153,7 +2153,7 @@ int hci_bdaddr_list_add_with_flags(struct list_head *list, bdaddr_t *bdaddr,

bacpy(&entry->bdaddr, bdaddr);
entry->bdaddr_type = type;
- bitmap_from_u64(entry->flags, flags);
+ bitmap_from_arr32(entry->flags, &flags, 32);

list_add(&entry->list, list);

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 74937a834648..b63025c70c2c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4519,7 +4519,8 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
cp->addr.type);

if (br_params) {
- bitmap_from_u64(br_params->flags, current_flags);
+ bitmap_from_arr32(br_params->flags, &current_flags,
+ __HCI_CONN_NUM_FLAGS);
status = MGMT_STATUS_SUCCESS;
} else {
bt_dev_warn(hdev, "No such BR/EDR device %pMR (0x%x)",
@@ -4531,7 +4532,7 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
if (params) {
DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);

- bitmap_from_u64(flags, current_flags);
+ bitmap_from_arr32(flags, &current_flags, __HCI_CONN_NUM_FLAGS);

/* Devices using RPAs can only be programmed in the
* acceptlist LL Privacy has been enable otherwise they
@@ -4546,7 +4547,7 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
goto unlock;
}

- bitmap_from_u64(params->flags, current_flags);
+ bitmap_from_arr32(params->flags, &current_flags, __HCI_CONN_NUM_FLAGS);
status = MGMT_STATUS_SUCCESS;

/* Update passive scan if HCI_CONN_FLAG_DEVICE_PRIVACY
--
2.32.0