* [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64
@ 2022-05-19 20:15 Luiz Augusto von Dentz
2022-05-19 21:02 ` [v2] " bluez.test.bot
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2022-05-19 20:15 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
bitmap_from_u64 expects at least 8 bytes to be declared since it doesn't
take any parameter regarding the number of bits causing the following
warnings:
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:270:2,
inlined from 'bitmap_from_u64' at include/linux/bitmap.h:622:2,
inlined from 'set_device_flags' at net/bluetooth/mgmt.c:4534:4:
include/linux/bitmap.h:261:9: warning: 'memcpy' forming offset [4, 7] is
out of the bounds [0, 4] of object 'flags' with type
'long unsigned int[1]' [-Warray-bounds]
261 | memcpy(dst, src, len);
| ^~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/arm/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)]
| ^~~~
In order to fix the above this initializes a variable using
DECLARE_BITMAP with the current_flags and then uses bitmap_subset to
check if the flags being set are a subset of hdev->conn_flags that way
all the checks are performed using bitmap APIs and conversion to u32
only happen when really needed.
Fixes: a9a347655d22 ("Bluetooth: MGMT: Add conditions for setting HCI_CONN_FLAG_REMOTE_WAKEUP")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Suggested-by: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/mgmt.c | 45 +++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 74937a834648..cd1b300b9be7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4476,9 +4476,16 @@ static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
static void device_flags_changed(struct sock *sk, struct hci_dev *hdev,
bdaddr_t *bdaddr, u8 bdaddr_type,
- u32 supported_flags, u32 current_flags)
+ unsigned long *flags)
{
struct mgmt_ev_device_flags_changed ev;
+ u32 supported_flags, current_flags = 0;
+
+ bitmap_to_arr32(&supported_flags, hdev->conn_flags,
+ __HCI_CONN_NUM_FLAGS);
+
+ if (flags)
+ bitmap_to_arr32(¤t_flags, flags, __HCI_CONN_NUM_FLAGS);
bacpy(&ev.addr.bdaddr, bdaddr);
ev.addr.type = bdaddr_type;
@@ -4495,19 +4502,15 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
struct bdaddr_list_with_flags *br_params;
struct hci_conn_params *params;
u8 status = MGMT_STATUS_INVALID_PARAMS;
- u32 supported_flags;
u32 current_flags = __le32_to_cpu(cp->current_flags);
+ DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS) = { current_flags };
bt_dev_dbg(hdev, "Set device flags %pMR (type 0x%x) = 0x%x",
- &cp->addr.bdaddr, cp->addr.type,
- __le32_to_cpu(current_flags));
+ &cp->addr.bdaddr, cp->addr.type, current_flags);
- bitmap_to_arr32(&supported_flags, hdev->conn_flags,
- __HCI_CONN_NUM_FLAGS);
-
- if ((supported_flags | current_flags) != supported_flags) {
- bt_dev_warn(hdev, "Bad flag given (0x%x) vs supported (0x%0x)",
- current_flags, supported_flags);
+ if (bitmap_subset(hdev->conn_flags, flags, __HCI_CONN_NUM_FLAGS)) {
+ bt_dev_warn(hdev, "Bad flag given (0x%lx) vs supported (0x%lx)",
+ *hdev->conn_flags, *flags);
goto done;
}
@@ -4519,7 +4522,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_copy(br_params->flags, flags,
+ __HCI_CONN_NUM_FLAGS);
status = MGMT_STATUS_SUCCESS;
} else {
bt_dev_warn(hdev, "No such BR/EDR device %pMR (0x%x)",
@@ -4529,10 +4533,6 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
le_addr_type(cp->addr.type));
if (params) {
- DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);
-
- bitmap_from_u64(flags, current_flags);
-
/* Devices using RPAs can only be programmed in the
* acceptlist LL Privacy has been enable otherwise they
* cannot mark HCI_CONN_FLAG_REMOTE_WAKEUP.
@@ -4546,7 +4546,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_copy(params->flags, flags, __HCI_CONN_NUM_FLAGS);
status = MGMT_STATUS_SUCCESS;
/* Update passive scan if HCI_CONN_FLAG_DEVICE_PRIVACY
@@ -4568,7 +4568,7 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
done:
if (status == MGMT_STATUS_SUCCESS)
device_flags_changed(sk, hdev, &cp->addr.bdaddr, cp->addr.type,
- supported_flags, current_flags);
+ flags);
return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_DEVICE_FLAGS, status,
&cp->addr, sizeof(cp->addr));
@@ -7079,10 +7079,8 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
{
struct mgmt_cp_add_device *cp = data;
u8 auto_conn, addr_type;
- struct hci_conn_params *params;
+ struct hci_conn_params *params = NULL;
int err;
- u32 current_flags = 0;
- u32 supported_flags;
bt_dev_dbg(hdev, "sock %p", sk);
@@ -7153,9 +7151,6 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
} else {
params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
addr_type);
- if (params)
- bitmap_to_arr32(¤t_flags, params->flags,
- __HCI_CONN_NUM_FLAGS);
}
err = hci_cmd_sync_queue(hdev, add_device_sync, NULL, NULL);
@@ -7164,10 +7159,8 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
added:
device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
- bitmap_to_arr32(&supported_flags, hdev->conn_flags,
- __HCI_CONN_NUM_FLAGS);
device_flags_changed(NULL, hdev, &cp->addr.bdaddr, cp->addr.type,
- supported_flags, current_flags);
+ params ? params->flags : NULL);
err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
MGMT_STATUS_SUCCESS, &cp->addr,
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64
2022-05-19 20:15 [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64 Luiz Augusto von Dentz
@ 2022-05-19 21:02 ` bluez.test.bot
0 siblings, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2022-05-19 21:02 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=643308
---Test result---
Test Summary:
CheckPatch FAIL 1.02 seconds
GitLint PASS 0.51 seconds
SubjectPrefix PASS 0.39 seconds
BuildKernel PASS 31.36 seconds
BuildKernel32 PASS 27.70 seconds
Incremental Build with patchesPASS 38.20 seconds
TestRunner: Setup PASS 470.62 seconds
TestRunner: l2cap-tester PASS 16.85 seconds
TestRunner: bnep-tester PASS 5.81 seconds
TestRunner: mgmt-tester PASS 99.22 seconds
TestRunner: rfcomm-tester PASS 9.17 seconds
TestRunner: sco-tester PASS 8.99 seconds
TestRunner: smp-tester PASS 8.99 seconds
TestRunner: userchan-tester PASS 5.94 seconds
Details
##############################
Test: CheckPatch - FAIL - 1.02 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
[v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64\WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#110:
4532 | DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);
total: 0 errors, 1 warnings, 0 checks, 107 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/12855993.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64
2022-05-19 18:09 ` Marcel Holtmann
@ 2022-05-19 20:11 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2022-05-19 20:11 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Thu, May 19, 2022 at 11:09 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > bitmap_from_u64 expects at least 8 bytes to be declared since it doesn't
> > take any parameter regarding the number of bits causing the following
> > warnings:
> >
> > 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:270:2,
> > inlined from 'bitmap_from_u64' at include/linux/bitmap.h:622:2,
> > inlined from 'set_device_flags' at net/bluetooth/mgmt.c:4534:4:
> > include/linux/bitmap.h:261:9: warning: 'memcpy' forming offset [4, 7] is
> > out of the bounds [0, 4] of object 'flags' with type
> > 'long unsigned int[1]' [-Warray-bounds]
> > 261 | memcpy(dst, src, len);
> > | ^~~~~~~~~~~~~~~~~~~~~
> > In file included from include/linux/kasan-checks.h:5,
> > from include/asm-generic/rwonce.h:26,
> > from ./arch/arm/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)]
> > | ^~~~
> >
> > In order to fix the above this initializes a variable using
> > DECLARE_BITMAP with the current_flags and then uses bitmap_subset to
> > check if the flags being set are a subset of hdev->conn_flags that way
> > all the checks are performed using bitmap APIs and conversion to u32
> > only happen when really needed.
> >
> > Fixes: a9a347655d22 ("Bluetooth: MGMT: Add conditions for setting HCI_CONN_FLAG_REMOTE_WAKEUP")
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Suggested-by: Yury Norov <yury.norov@gmail.com>
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > net/bluetooth/mgmt.c | 43 ++++++++++++++++++-------------------------
> > 1 file changed, 18 insertions(+), 25 deletions(-)
>
> what is up with the kbot issues?
Will fix that.
> Regards
>
> Marcel
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64
2022-05-16 20:35 [PATCH v2] " Luiz Augusto von Dentz
2022-05-17 3:09 ` kernel test robot
@ 2022-05-19 18:09 ` Marcel Holtmann
2022-05-19 20:11 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2022-05-19 18:09 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
> bitmap_from_u64 expects at least 8 bytes to be declared since it doesn't
> take any parameter regarding the number of bits causing the following
> warnings:
>
> 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:270:2,
> inlined from 'bitmap_from_u64' at include/linux/bitmap.h:622:2,
> inlined from 'set_device_flags' at net/bluetooth/mgmt.c:4534:4:
> include/linux/bitmap.h:261:9: warning: 'memcpy' forming offset [4, 7] is
> out of the bounds [0, 4] of object 'flags' with type
> 'long unsigned int[1]' [-Warray-bounds]
> 261 | memcpy(dst, src, len);
> | ^~~~~~~~~~~~~~~~~~~~~
> In file included from include/linux/kasan-checks.h:5,
> from include/asm-generic/rwonce.h:26,
> from ./arch/arm/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)]
> | ^~~~
>
> In order to fix the above this initializes a variable using
> DECLARE_BITMAP with the current_flags and then uses bitmap_subset to
> check if the flags being set are a subset of hdev->conn_flags that way
> all the checks are performed using bitmap APIs and conversion to u32
> only happen when really needed.
>
> Fixes: a9a347655d22 ("Bluetooth: MGMT: Add conditions for setting HCI_CONN_FLAG_REMOTE_WAKEUP")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Suggested-by: Yury Norov <yury.norov@gmail.com>
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> net/bluetooth/mgmt.c | 43 ++++++++++++++++++-------------------------
> 1 file changed, 18 insertions(+), 25 deletions(-)
what is up with the kbot issues?
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64
2022-05-16 20:35 [PATCH v2] " Luiz Augusto von Dentz
@ 2022-05-17 3:09 ` kernel test robot
2022-05-19 18:09 ` Marcel Holtmann
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2022-05-17 3:09 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth; +Cc: llvm, kbuild-all
Hi Luiz,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20220516]
[cannot apply to bluetooth/master v5.18-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Luiz-Augusto-von-Dentz/Bluetooth-MGMT-Fix-uses-of-bitmap_from_u64/20220517-050324
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: hexagon-randconfig-r041-20220516 (https://download.01.org/0day-ci/archive/20220517/202205171145.9ekbbpAa-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 853fa8ee225edf2d0de94b0dcbd31bea916e825e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/15b968f930769afbaabbfb85c2cefcbc30506a7b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Luiz-Augusto-von-Dentz/Bluetooth-MGMT-Fix-uses-of-bitmap_from_u64/20220517-050324
git checkout 15b968f930769afbaabbfb85c2cefcbc30506a7b
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/bluetooth/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> net/bluetooth/mgmt.c:7113:7: warning: variable 'params' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (err)
^~~
net/bluetooth/mgmt.c:7163:9: note: uninitialized use occurs here
params ? params->flags : NULL);
^~~~~~
net/bluetooth/mgmt.c:7113:3: note: remove the 'if' if its condition is always true
if (err)
^~~~~~~~
net/bluetooth/mgmt.c:7082:32: note: initialize the variable 'params' to silence this warning
struct hci_conn_params *params;
^
= NULL
1 warning generated.
vim +7113 net/bluetooth/mgmt.c
e8907f76544ffe Luiz Augusto von Dentz 2021-10-27 7076
2faade53e65f27 Marcel Holtmann 2014-06-29 7077 static int add_device(struct sock *sk, struct hci_dev *hdev,
2faade53e65f27 Marcel Holtmann 2014-06-29 7078 void *data, u16 len)
2faade53e65f27 Marcel Holtmann 2014-06-29 7079 {
2faade53e65f27 Marcel Holtmann 2014-06-29 7080 struct mgmt_cp_add_device *cp = data;
2faade53e65f27 Marcel Holtmann 2014-06-29 7081 u8 auto_conn, addr_type;
4c54bf2b093bb2 Abhishek Pandit-Subedi 2020-06-17 7082 struct hci_conn_params *params;
2faade53e65f27 Marcel Holtmann 2014-06-29 7083 int err;
2faade53e65f27 Marcel Holtmann 2014-06-29 7084
181d695352305c Marcel Holtmann 2020-05-06 7085 bt_dev_dbg(hdev, "sock %p", sk);
2faade53e65f27 Marcel Holtmann 2014-06-29 7086
6659358efe617b Johan Hedberg 2014-07-09 7087 if (!bdaddr_type_is_valid(cp->addr.type) ||
2faade53e65f27 Marcel Holtmann 2014-06-29 7088 !bacmp(&cp->addr.bdaddr, BDADDR_ANY))
2a1afb5ac8d580 Johan Hedberg 2015-03-06 7089 return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
2faade53e65f27 Marcel Holtmann 2014-06-29 7090 MGMT_STATUS_INVALID_PARAMS,
2faade53e65f27 Marcel Holtmann 2014-06-29 7091 &cp->addr, sizeof(cp->addr));
2faade53e65f27 Marcel Holtmann 2014-06-29 7092
4b9e7e7516135b Marcel Holtmann 2014-07-23 7093 if (cp->action != 0x00 && cp->action != 0x01 && cp->action != 0x02)
2a1afb5ac8d580 Johan Hedberg 2015-03-06 7094 return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
2faade53e65f27 Marcel Holtmann 2014-06-29 7095 MGMT_STATUS_INVALID_PARAMS,
2faade53e65f27 Marcel Holtmann 2014-06-29 7096 &cp->addr, sizeof(cp->addr));
2faade53e65f27 Marcel Holtmann 2014-06-29 7097
2faade53e65f27 Marcel Holtmann 2014-06-29 7098 hci_dev_lock(hdev);
2faade53e65f27 Marcel Holtmann 2014-06-29 7099
6659358efe617b Johan Hedberg 2014-07-09 7100 if (cp->addr.type == BDADDR_BREDR) {
4b9e7e7516135b Marcel Holtmann 2014-07-23 7101 /* Only incoming connections action is supported for now */
6659358efe617b Johan Hedberg 2014-07-09 7102 if (cp->action != 0x01) {
51d7a94d56f842 Johan Hedberg 2015-11-11 7103 err = mgmt_cmd_complete(sk, hdev->id,
51d7a94d56f842 Johan Hedberg 2015-11-11 7104 MGMT_OP_ADD_DEVICE,
51d7a94d56f842 Johan Hedberg 2015-11-11 7105 MGMT_STATUS_INVALID_PARAMS,
51d7a94d56f842 Johan Hedberg 2015-11-11 7106 &cp->addr, sizeof(cp->addr));
6659358efe617b Johan Hedberg 2014-07-09 7107 goto unlock;
6659358efe617b Johan Hedberg 2014-07-09 7108 }
6659358efe617b Johan Hedberg 2014-07-09 7109
3d4f9c00492b4e Archie Pusaka 2021-06-04 7110 err = hci_bdaddr_list_add_with_flags(&hdev->accept_list,
8baaa4038edbff Abhishek Pandit-Subedi 2020-06-17 7111 &cp->addr.bdaddr,
8baaa4038edbff Abhishek Pandit-Subedi 2020-06-17 7112 cp->addr.type, 0);
6659358efe617b Johan Hedberg 2014-07-09 @7113 if (err)
6659358efe617b Johan Hedberg 2014-07-09 7114 goto unlock;
a397407f266f8d Johan Hedberg 2014-07-09 7115
01b1cb87d37fb1 Johan Hedberg 2015-11-16 7116 hci_req_update_scan(hdev);
a397407f266f8d Johan Hedberg 2014-07-09 7117
6659358efe617b Johan Hedberg 2014-07-09 7118 goto added;
6659358efe617b Johan Hedberg 2014-07-09 7119 }
6659358efe617b Johan Hedberg 2014-07-09 7120
85813a7ec774b9 Johan Hedberg 2015-10-21 7121 addr_type = le_addr_type(cp->addr.type);
2faade53e65f27 Marcel Holtmann 2014-06-29 7122
4b9e7e7516135b Marcel Holtmann 2014-07-23 7123 if (cp->action == 0x02)
2faade53e65f27 Marcel Holtmann 2014-06-29 7124 auto_conn = HCI_AUTO_CONN_ALWAYS;
4b9e7e7516135b Marcel Holtmann 2014-07-23 7125 else if (cp->action == 0x01)
4b9e7e7516135b Marcel Holtmann 2014-07-23 7126 auto_conn = HCI_AUTO_CONN_DIRECT;
2faade53e65f27 Marcel Holtmann 2014-06-29 7127 else
a3451d279f839d Johan Hedberg 2014-07-02 7128 auto_conn = HCI_AUTO_CONN_REPORT;
2faade53e65f27 Marcel Holtmann 2014-06-29 7129
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7130 /* Kernel internally uses conn_params with resolvable private
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7131 * address, but Add Device allows only identity addresses.
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7132 * Make sure it is enforced before calling
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7133 * hci_conn_params_lookup.
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7134 */
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7135 if (!hci_is_identity_address(&cp->addr.bdaddr, addr_type)) {
51d7a94d56f842 Johan Hedberg 2015-11-11 7136 err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
51d7a94d56f842 Johan Hedberg 2015-11-11 7137 MGMT_STATUS_INVALID_PARAMS,
51d7a94d56f842 Johan Hedberg 2015-11-11 7138 &cp->addr, sizeof(cp->addr));
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7139 goto unlock;
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7140 }
9a0a8a8e852dab Jakub Pawlowski 2015-07-20 7141
bf5b3c8be07905 Marcel Holtmann 2014-06-30 7142 /* If the connection parameters don't exist for this device,
bf5b3c8be07905 Marcel Holtmann 2014-06-30 7143 * they will be created and configured with defaults.
bf5b3c8be07905 Marcel Holtmann 2014-06-30 7144 */
51d7a94d56f842 Johan Hedberg 2015-11-11 7145 if (hci_conn_params_set(hdev, &cp->addr.bdaddr, addr_type,
d06b50ce14119a Marcel Holtmann 2014-07-01 7146 auto_conn) < 0) {
51d7a94d56f842 Johan Hedberg 2015-11-11 7147 err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
51d7a94d56f842 Johan Hedberg 2015-11-11 7148 MGMT_STATUS_FAILED, &cp->addr,
51d7a94d56f842 Johan Hedberg 2015-11-11 7149 sizeof(cp->addr));
2faade53e65f27 Marcel Holtmann 2014-06-29 7150 goto unlock;
4c54bf2b093bb2 Abhishek Pandit-Subedi 2020-06-17 7151 } else {
4c54bf2b093bb2 Abhishek Pandit-Subedi 2020-06-17 7152 params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
4c54bf2b093bb2 Abhishek Pandit-Subedi 2020-06-17 7153 addr_type);
2faade53e65f27 Marcel Holtmann 2014-06-29 7154 }
2faade53e65f27 Marcel Holtmann 2014-06-29 7155
e8907f76544ffe Luiz Augusto von Dentz 2021-10-27 7156 err = hci_cmd_sync_queue(hdev, add_device_sync, NULL, NULL);
e8907f76544ffe Luiz Augusto von Dentz 2021-10-27 7157 if (err < 0)
e8907f76544ffe Luiz Augusto von Dentz 2021-10-27 7158 goto unlock;
51d7a94d56f842 Johan Hedberg 2015-11-11 7159
6659358efe617b Johan Hedberg 2014-07-09 7160 added:
8afef092a192cb Marcel Holtmann 2014-06-29 7161 device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
4c54bf2b093bb2 Abhishek Pandit-Subedi 2020-06-17 7162 device_flags_changed(NULL, hdev, &cp->addr.bdaddr, cp->addr.type,
15b968f930769a Luiz Augusto von Dentz 2022-05-16 7163 params ? params->flags : NULL);
8afef092a192cb Marcel Holtmann 2014-06-29 7164
51d7a94d56f842 Johan Hedberg 2015-11-11 7165 err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
51d7a94d56f842 Johan Hedberg 2015-11-11 7166 MGMT_STATUS_SUCCESS, &cp->addr,
51d7a94d56f842 Johan Hedberg 2015-11-11 7167 sizeof(cp->addr));
2faade53e65f27 Marcel Holtmann 2014-06-29 7168
2faade53e65f27 Marcel Holtmann 2014-06-29 7169 unlock:
2faade53e65f27 Marcel Holtmann 2014-06-29 7170 hci_dev_unlock(hdev);
2faade53e65f27 Marcel Holtmann 2014-06-29 7171 return err;
2faade53e65f27 Marcel Holtmann 2014-06-29 7172 }
2faade53e65f27 Marcel Holtmann 2014-06-29 7173
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64
@ 2022-05-16 20:35 Luiz Augusto von Dentz
2022-05-17 3:09 ` kernel test robot
2022-05-19 18:09 ` Marcel Holtmann
0 siblings, 2 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2022-05-16 20:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
bitmap_from_u64 expects at least 8 bytes to be declared since it doesn't
take any parameter regarding the number of bits causing the following
warnings:
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:270:2,
inlined from 'bitmap_from_u64' at include/linux/bitmap.h:622:2,
inlined from 'set_device_flags' at net/bluetooth/mgmt.c:4534:4:
include/linux/bitmap.h:261:9: warning: 'memcpy' forming offset [4, 7] is
out of the bounds [0, 4] of object 'flags' with type
'long unsigned int[1]' [-Warray-bounds]
261 | memcpy(dst, src, len);
| ^~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/kasan-checks.h:5,
from include/asm-generic/rwonce.h:26,
from ./arch/arm/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)]
| ^~~~
In order to fix the above this initializes a variable using
DECLARE_BITMAP with the current_flags and then uses bitmap_subset to
check if the flags being set are a subset of hdev->conn_flags that way
all the checks are performed using bitmap APIs and conversion to u32
only happen when really needed.
Fixes: a9a347655d22 ("Bluetooth: MGMT: Add conditions for setting HCI_CONN_FLAG_REMOTE_WAKEUP")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Suggested-by: Yury Norov <yury.norov@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
net/bluetooth/mgmt.c | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 74937a834648..24a779b706b5 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4476,9 +4476,16 @@ static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
static void device_flags_changed(struct sock *sk, struct hci_dev *hdev,
bdaddr_t *bdaddr, u8 bdaddr_type,
- u32 supported_flags, u32 current_flags)
+ unsigned long *flags)
{
struct mgmt_ev_device_flags_changed ev;
+ u32 supported_flags, current_flags = 0;
+
+ bitmap_to_arr32(&supported_flags, hdev->conn_flags,
+ __HCI_CONN_NUM_FLAGS);
+
+ if (flags)
+ bitmap_to_arr32(¤t_flags, flags, __HCI_CONN_NUM_FLAGS);
bacpy(&ev.addr.bdaddr, bdaddr);
ev.addr.type = bdaddr_type;
@@ -4495,19 +4502,15 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
struct bdaddr_list_with_flags *br_params;
struct hci_conn_params *params;
u8 status = MGMT_STATUS_INVALID_PARAMS;
- u32 supported_flags;
u32 current_flags = __le32_to_cpu(cp->current_flags);
+ DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS) = { current_flags };
bt_dev_dbg(hdev, "Set device flags %pMR (type 0x%x) = 0x%x",
- &cp->addr.bdaddr, cp->addr.type,
- __le32_to_cpu(current_flags));
+ &cp->addr.bdaddr, cp->addr.type, current_flags);
- bitmap_to_arr32(&supported_flags, hdev->conn_flags,
- __HCI_CONN_NUM_FLAGS);
-
- if ((supported_flags | current_flags) != supported_flags) {
- bt_dev_warn(hdev, "Bad flag given (0x%x) vs supported (0x%0x)",
- current_flags, supported_flags);
+ if (bitmap_subset(hdev->conn_flags, flags, __HCI_CONN_NUM_FLAGS)) {
+ bt_dev_warn(hdev, "Bad flag given (0x%lx) vs supported (0x%lx)",
+ *hdev->conn_flags, *flags);
goto done;
}
@@ -4519,7 +4522,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_copy(br_params->flags, flags,
+ __HCI_CONN_NUM_FLAGS);
status = MGMT_STATUS_SUCCESS;
} else {
bt_dev_warn(hdev, "No such BR/EDR device %pMR (0x%x)",
@@ -4529,10 +4533,6 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
le_addr_type(cp->addr.type));
if (params) {
- DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);
-
- bitmap_from_u64(flags, current_flags);
-
/* Devices using RPAs can only be programmed in the
* acceptlist LL Privacy has been enable otherwise they
* cannot mark HCI_CONN_FLAG_REMOTE_WAKEUP.
@@ -4546,7 +4546,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_copy(params->flags, flags, __HCI_CONN_NUM_FLAGS);
status = MGMT_STATUS_SUCCESS;
/* Update passive scan if HCI_CONN_FLAG_DEVICE_PRIVACY
@@ -4568,7 +4568,7 @@ static int set_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
done:
if (status == MGMT_STATUS_SUCCESS)
device_flags_changed(sk, hdev, &cp->addr.bdaddr, cp->addr.type,
- supported_flags, current_flags);
+ flags);
return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_DEVICE_FLAGS, status,
&cp->addr, sizeof(cp->addr));
@@ -7081,8 +7081,6 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
u8 auto_conn, addr_type;
struct hci_conn_params *params;
int err;
- u32 current_flags = 0;
- u32 supported_flags;
bt_dev_dbg(hdev, "sock %p", sk);
@@ -7153,9 +7151,6 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
} else {
params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr,
addr_type);
- if (params)
- bitmap_to_arr32(¤t_flags, params->flags,
- __HCI_CONN_NUM_FLAGS);
}
err = hci_cmd_sync_queue(hdev, add_device_sync, NULL, NULL);
@@ -7164,10 +7159,8 @@ static int add_device(struct sock *sk, struct hci_dev *hdev,
added:
device_added(sk, hdev, &cp->addr.bdaddr, cp->addr.type, cp->action);
- bitmap_to_arr32(&supported_flags, hdev->conn_flags,
- __HCI_CONN_NUM_FLAGS);
device_flags_changed(NULL, hdev, &cp->addr.bdaddr, cp->addr.type,
- supported_flags, current_flags);
+ params ? params->flags : NULL);
err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_DEVICE,
MGMT_STATUS_SUCCESS, &cp->addr,
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-19 21:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-19 20:15 [PATCH v2] Bluetooth: MGMT: Fix uses of bitmap_from_u64 Luiz Augusto von Dentz
2022-05-19 21:02 ` [v2] " bluez.test.bot
-- strict thread matches above, loose matches on Subject: below --
2022-05-16 20:35 [PATCH v2] " Luiz Augusto von Dentz
2022-05-17 3:09 ` kernel test robot
2022-05-19 18:09 ` Marcel Holtmann
2022-05-19 20:11 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.