diff mbox series

[v8,1/3] bluetooth: msft: Handle MSFT Monitor Device Event

Message ID 20211215203919.v8.1.Ic0a40b84dee3825302890aaea690e73165c71820@changeid (mailing list archive)
State Superseded
Headers show
Series [v8,1/3] bluetooth: msft: Handle MSFT Monitor Device Event | expand

Checks

Context Check Description
tedd_an/checkpatch fail [v8,1/3] bluetooth: msft: Handle MSFT Monitor Device Event\WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message #258: FILE: net/bluetooth/msft.c:629: + if (!dev) { + bt_dev_err(hdev, "MSFT vendor event %u: no memory", total: 0 errors, 1 warnings, 0 checks, 228 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/12679931.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.
tedd_an/gitlint success Gitlint PASS
tedd_an/buildkernel fail Build Kernel make FAIL: In file included from ./include/linux/wait.h:7, from ./include/linux/poll.h:8, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: net/bluetooth/msft.c: In function ‘msft_le_cancel_monitor_advertisement_cb’: net/bluetooth/msft.c:306:28: error: ‘dev’ undeclared (first use in this function); did you mean ‘hdev’? 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~ ./include/linux/list.h:717:7: note: in definition of macro ‘list_for_each_entry_safe’ 717 | for (pos = list_first_entry(head, typeof(*pos), member), \ | ^~~ net/bluetooth/msft.c:306:28: note: each undeclared identifier is reported only once for each function it appears in 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~ ./include/linux/list.h:717:7: note: in definition of macro ‘list_for_each_entry_safe’ 717 | for (pos = list_first_entry(head, typeof(*pos), member), \ | ^~~ In file included from ./include/linux/container_of.h:5, from ./include/linux/kernel.h:12, from ./arch/x86/include/asm/percpu.h:27, from ./arch/x86/include/asm/current.h:6, from ./arch/x86/include/asm/processor.h:17, from ./arch/x86/include/asm/timex.h:5, from ./include/linux/timex.h:65, from ./include/linux/time32.h:13, from ./include/linux/time.h:60, from ./include/linux/ktime.h:24, from ./include/linux/poll.h:7, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: ././include/linux/compiler_types.h:276:27: error: expression in static assertion is not an integer 276 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’ 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ ./include/linux/container_of.h:19:2: note: in expansion of macro ‘static_assert’ 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ ./include/linux/container_of.h:19:16: note: in expansion of macro ‘__same_type’ 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ ./include/linux/list.h:513:2: note: in expansion of macro ‘container_of’ 513 | container_of(ptr, type, member) | ^~~~~~~~~~~~ ./include/linux/list.h:524:2: note: in expansion of macro ‘list_entry’ 524 | list_entry((ptr)->next, type, member) | ^~~~~~~~~~ ./include/linux/list.h:717:13: note: in expansion of macro ‘list_first_entry’ 717 | for (pos = list_first_entry(head, typeof(*pos), member), \ | ^~~~~~~~~~~~~~~~ net/bluetooth/msft.c:306:3: note: in expansion of macro ‘list_for_each_entry_safe’ 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from ./include/linux/wait.h:7, from ./include/linux/poll.h:8, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: net/bluetooth/msft.c:306:33: error: ‘tmp’ undeclared (first use in this function); did you mean ‘tm’? 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~ ./include/linux/list.h:718:3: note: in definition of macro ‘list_for_each_entry_safe’ 718 | n = list_next_entry(pos, member); \ | ^ In file included from ./include/linux/container_of.h:5, from ./include/linux/kernel.h:12, from ./arch/x86/include/asm/percpu.h:27, from ./arch/x86/include/asm/current.h:6, from ./arch/x86/include/asm/processor.h:17, from ./arch/x86/include/asm/timex.h:5, from ./include/linux/timex.h:65, from ./include/linux/time32.h:13, from ./include/linux/time.h:60, from ./include/linux/ktime.h:24, from ./include/linux/poll.h:7, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: ././include/linux/compiler_types.h:276:27: error: expression in static assertion is not an integer 276 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’ 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ ./include/linux/container_of.h:19:2: note: in expansion of macro ‘static_assert’ 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ ./include/linux/container_of.h:19:16: note: in expansion of macro ‘__same_type’ 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ ./include/linux/list.h:513:2: note: in expansion of macro ‘container_of’ 513 | container_of(ptr, type, member) | ^~~~~~~~~~~~ ./include/linux/list.h:557:2: note: in expansion of macro ‘list_entry’ 557 | list_entry((pos)->member.next, typeof(*(pos)), member) | ^~~~~~~~~~ ./include/linux/list.h:718:7: note: in expansion of macro ‘list_next_entry’ 718 | n = list_next_entry(pos, member); \ | ^~~~~~~~~~~~~~~ net/bluetooth/msft.c:306:3: note: in expansion of macro ‘list_for_each_entry_safe’ 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from ./include/linux/wait.h:7, from ./include/linux/poll.h:8, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: ./include/linux/list.h:717:57: warning: left-hand operand of comma expression has no effect [-Wunused-value] 717 | for (pos = list_first_entry(head, typeof(*pos), member), \ | ^ net/bluetooth/msft.c:306:3: note: in expansion of macro ‘list_for_each_entry_safe’ 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from ./include/linux/container_of.h:5, from ./include/linux/kernel.h:12, from ./arch/x86/include/asm/percpu.h:27, from ./arch/x86/include/asm/current.h:6, from ./arch/x86/include/asm/processor.h:17, from ./arch/x86/include/asm/timex.h:5, from ./include/linux/timex.h:65, from ./include/linux/time32.h:13, from ./include/linux/time.h:60, from ./include/linux/ktime.h:24, from ./include/linux/poll.h:7, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: ././include/linux/compiler_types.h:276:27: error: expression in static assertion is not an integer 276 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’ 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~ ./include/linux/container_of.h:19:2: note: in expansion of macro ‘static_assert’ 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~~~ ./include/linux/container_of.h:19:16: note: in expansion of macro ‘__same_type’ 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~~~~~~~~~ ./include/linux/list.h:513:2: note: in expansion of macro ‘container_of’ 513 | container_of(ptr, type, member) | ^~~~~~~~~~~~ ./include/linux/list.h:557:2: note: in expansion of macro ‘list_entry’ 557 | list_entry((pos)->member.next, typeof(*(pos)), member) | ^~~~~~~~~~ ./include/linux/list.h:720:20: note: in expansion of macro ‘list_next_entry’ 720 | pos = n, n = list_next_entry(n, member)) | ^~~~~~~~~~~~~~~ net/bluetooth/msft.c:306:3: note: in expansion of macro ‘list_for_each_entry_safe’ 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from ./include/linux/wait.h:7, from ./include/linux/poll.h:8, from ./include/net/bluetooth/bluetooth.h:28, from net/bluetooth/msft.c:6: ./include/linux/list.h:720:14: warning: left-hand operand of comma expression has no effect [-Wunused-value] 720 | pos = n, n = list_next_entry(n, member)) | ^ net/bluetooth/msft.c:306:3: note: in expansion of macro ‘list_for_each_entry_safe’ 306 | list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, | ^~~~~~~~~~~~~~~~~~~~~~~~ make[2]: *** [scripts/Makefile.build:287: net/bluetooth/msft.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [scripts/Makefile.build:549: net/bluetooth] Error 2 make: *** [Makefile:1846: net] Error 2 make: *** Waiting for unfinished jobs....
tedd_an/incremental_build pending Incremental Build Kernel SKIP(Build Fail)
tedd_an/testrunnersetup fail Test Runner Setup Build Kernel FAIL

Commit Message

Manish Mandlik Dec. 16, 2021, 4:39 a.m. UTC
Whenever the controller starts/stops monitoring a bt device, it sends
MSFT Monitor Device event. Add handler to read this vendor event.

Test performed:
- Verified by logs that the MSFT Monitor Device event is received from
  the controller whenever it starts/stops monitoring a device.

Signed-off-by: Manish Mandlik <mmandlik@google.com>
Reviewed-by: Miao-chen Chou <mcchou@google.com>
---
Hello Bt-Maintainers,

As mentioned in the bluez patch series [1], we need to capture the 'MSFT
Monitor Device' event from the controller and pass on the necessary
information to the bluetoothd.

This is required to further optimize the power consumption by avoiding
handling of RSSI thresholds and timeouts in the user space and let the
controller do the RSSI tracking.

This patch series adds support to read the MSFT vendor event
HCI_VS_MSFT_LE_Monitor_Device_Event and introduces new MGMT events
MGMT_EV_ADV_MONITOR_DEVICE_FOUND and MGMT_EV_ADV_MONITOR_DEVICE_LOST to
indicate that the controller has started/stopped tracking a particular
device.

Please let me know what you think about this or if you have any further
questions.

[1] https://patchwork.kernel.org/project/bluetooth/list/?series=583423

Thanks,
Manish.

Changes in v8:
- Fix use-after-free in msft_le_cancel_monitor_advertisement_cb().
- Use skb_pull_data() instead of skb_pull().

Changes in v6:
- Fix compiler warning bt_dev_err() missing argument.

Changes in v5:
- Split v4 into two patches.
- Buffer controller Device Found event and maintain the device tracking
  state in the kernel.

Changes in v4:
- Add Advertisement Monitor Device Found event and update addr type.

Changes in v3:
- Discard changes to the Device Found event and notify bluetoothd only
  when the controller stops monitoring the device via new Device Lost
  event.

Changes in v2:
- Instead of creating a new 'Device Tracking' event, add a flag 'Device
  Tracked' in the existing 'Device Found' event and add a new 'Device
  Lost' event to indicate that the controller has stopped tracking that
  device.

 include/net/bluetooth/hci_core.h |  11 +++
 net/bluetooth/hci_core.c         |   1 +
 net/bluetooth/msft.c             | 149 +++++++++++++++++++++++++++++--
 3 files changed, 153 insertions(+), 8 deletions(-)

Comments

kernel test robot Dec. 16, 2021, 10:08 a.m. UTC | #1
Hi Manish,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on next-20211215]
[cannot apply to bluetooth/master v5.16-rc5]
[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/0day-ci/linux/commits/Manish-Mandlik/bluetooth-msft-Handle-MSFT-Monitor-Device-Event/20211216-124056
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: arc-randconfig-r043-20211216 (https://download.01.org/0day-ci/archive/20211216/202112161736.WMgjxclO-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.2.0
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/0day-ci/linux/commit/db0604d7b0d0308963bdc1465b486be11d5fdcb6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Manish-Mandlik/bluetooth-msft-Handle-MSFT-Monitor-Device-Event/20211216-124056
        git checkout db0604d7b0d0308963bdc1465b486be11d5fdcb6
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc 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 errors (new ones prefixed by >>):

   In file included from include/linux/wait.h:7,
                    from include/linux/poll.h:8,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
   net/bluetooth/msft.c: In function 'msft_le_cancel_monitor_advertisement_cb':
>> net/bluetooth/msft.c:306:42: error: 'dev' undeclared (first use in this function); did you mean 'hdev'?
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                                          ^~~
   include/linux/list.h:717:14: note: in definition of macro 'list_for_each_entry_safe'
     717 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |              ^~~
   net/bluetooth/msft.c:306:42: note: each undeclared identifier is reported only once for each function it appears in
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                                          ^~~
   include/linux/list.h:717:14: note: in definition of macro 'list_for_each_entry_safe'
     717 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |              ^~~
   In file included from arch/arc/include/asm/cache.h:28,
                    from include/linux/cache.h:6,
                    from include/linux/time.h:5,
                    from include/linux/ktime.h:24,
                    from include/linux/poll.h:7,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
   include/linux/compiler_types.h:276:27: error: expression in static assertion is not an integer
     276 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
      19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
      19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |                       ^~~~~~~~~~~
   include/linux/list.h:513:9: note: in expansion of macro 'container_of'
     513 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:524:9: note: in expansion of macro 'list_entry'
     524 |         list_entry((ptr)->next, type, member)
         |         ^~~~~~~~~~
   include/linux/list.h:717:20: note: in expansion of macro 'list_first_entry'
     717 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |                    ^~~~~~~~~~~~~~~~
   net/bluetooth/msft.c:306:17: note: in expansion of macro 'list_for_each_entry_safe'
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/wait.h:7,
                    from include/linux/poll.h:8,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
>> net/bluetooth/msft.c:306:47: error: 'tmp' undeclared (first use in this function); did you mean 'tm'?
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                                               ^~~
   include/linux/list.h:718:17: note: in definition of macro 'list_for_each_entry_safe'
     718 |                 n = list_next_entry(pos, member);                       \
         |                 ^
   In file included from arch/arc/include/asm/cache.h:28,
                    from include/linux/cache.h:6,
                    from include/linux/time.h:5,
                    from include/linux/ktime.h:24,
                    from include/linux/poll.h:7,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
   include/linux/compiler_types.h:276:27: error: expression in static assertion is not an integer
     276 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
      19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
      19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |                       ^~~~~~~~~~~
   include/linux/list.h:513:9: note: in expansion of macro 'container_of'
     513 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:557:9: note: in expansion of macro 'list_entry'
     557 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:718:21: note: in expansion of macro 'list_next_entry'
     718 |                 n = list_next_entry(pos, member);                       \
         |                     ^~~~~~~~~~~~~~~
   net/bluetooth/msft.c:306:17: note: in expansion of macro 'list_for_each_entry_safe'
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/wait.h:7,
                    from include/linux/poll.h:8,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
   include/linux/list.h:717:64: warning: left-hand operand of comma expression has no effect [-Wunused-value]
     717 |         for (pos = list_first_entry(head, typeof(*pos), member),        \
         |                                                                ^
   net/bluetooth/msft.c:306:17: note: in expansion of macro 'list_for_each_entry_safe'
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from arch/arc/include/asm/cache.h:28,
                    from include/linux/cache.h:6,
                    from include/linux/time.h:5,
                    from include/linux/ktime.h:24,
                    from include/linux/poll.h:7,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
   include/linux/compiler_types.h:276:27: error: expression in static assertion is not an integer
     276 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
         |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
      78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
         |                                                        ^~~~
   include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
      19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |         ^~~~~~~~~~~~~
   include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
      19 |         static_assert(__same_type(*(ptr), ((type *)0)->member) ||       \
         |                       ^~~~~~~~~~~
   include/linux/list.h:513:9: note: in expansion of macro 'container_of'
     513 |         container_of(ptr, type, member)
         |         ^~~~~~~~~~~~
   include/linux/list.h:557:9: note: in expansion of macro 'list_entry'
     557 |         list_entry((pos)->member.next, typeof(*(pos)), member)
         |         ^~~~~~~~~~
   include/linux/list.h:720:27: note: in expansion of macro 'list_next_entry'
     720 |              pos = n, n = list_next_entry(n, member))
         |                           ^~~~~~~~~~~~~~~
   net/bluetooth/msft.c:306:17: note: in expansion of macro 'list_for_each_entry_safe'
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/wait.h:7,
                    from include/linux/poll.h:8,
                    from include/net/bluetooth/bluetooth.h:28,
                    from net/bluetooth/msft.c:6:
   include/linux/list.h:720:21: warning: left-hand operand of comma expression has no effect [-Wunused-value]
     720 |              pos = n, n = list_next_entry(n, member))
         |                     ^
   net/bluetooth/msft.c:306:17: note: in expansion of macro 'list_for_each_entry_safe'
     306 |                 list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~


vim +306 net/bluetooth/msft.c

   265	
   266	static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
   267							    u8 status, u16 opcode,
   268							    struct sk_buff *skb)
   269	{
   270		struct msft_cp_le_cancel_monitor_advertisement *cp;
   271		struct msft_rp_le_cancel_monitor_advertisement *rp;
   272		struct adv_monitor *monitor;
   273		struct msft_monitor_advertisement_handle_data *handle_data;
   274		struct msft_data *msft = hdev->msft_data;
   275		int err;
   276		bool pending;
   277	
   278		if (status)
   279			goto done;
   280	
   281		rp = (struct msft_rp_le_cancel_monitor_advertisement *)skb->data;
   282		if (skb->len < sizeof(*rp)) {
   283			status = HCI_ERROR_UNSPECIFIED;
   284			goto done;
   285		}
   286	
   287		hci_dev_lock(hdev);
   288	
   289		cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
   290		handle_data = msft_find_handle_data(hdev, cp->handle, false);
   291	
   292		if (handle_data) {
   293			monitor = idr_find(&hdev->adv_monitors_idr,
   294					   handle_data->mgmt_handle);
   295	
   296			if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
   297				monitor->state = ADV_MONITOR_STATE_REGISTERED;
   298	
   299			/* Do not free the monitor if it is being removed due to
   300			 * suspend. It will be re-monitored on resume.
   301			 */
   302			if (monitor && !msft->suspending)
   303				hci_free_adv_monitor(hdev, monitor);
   304	
   305			/* Clear any monitored devices by this Adv Monitor */
 > 306			list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
   307						 list) {
   308				if (dev->handle == handle_data->mgmt_handle) {
   309					list_del(&dev->list);
   310					kfree(dev);
   311				}
   312			}
   313	
   314			list_del(&handle_data->list);
   315			kfree(handle_data);
   316		}
   317	
   318		/* If remove all monitors is required, we need to continue the process
   319		 * here because the earlier it was paused when waiting for the
   320		 * response from controller.
   321		 */
   322		if (msft->pending_remove_handle == 0) {
   323			pending = hci_remove_all_adv_monitor(hdev, &err);
   324			if (pending) {
   325				hci_dev_unlock(hdev);
   326				return;
   327			}
   328	
   329			if (err)
   330				status = HCI_ERROR_UNSPECIFIED;
   331		}
   332	
   333		hci_dev_unlock(hdev);
   334	
   335	done:
   336		if (!msft->suspending)
   337			hci_remove_adv_monitor_complete(hdev, status);
   338	}
   339	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4d69dcfebd63..c2a8b1163c30 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -258,6 +258,15 @@  struct adv_info {
 
 #define HCI_ADV_TX_POWER_NO_PREFERENCE 0x7F
 
+struct monitored_device {
+	struct list_head list;
+
+	bdaddr_t bdaddr;
+	__u8     addr_type;
+	__u16    handle;
+	bool     notified;
+};
+
 struct adv_pattern {
 	struct list_head list;
 	__u8 ad_type;
@@ -590,6 +599,8 @@  struct hci_dev {
 
 	struct delayed_work	interleave_scan;
 
+	struct list_head	monitored_devices;
+
 #if IS_ENABLED(CONFIG_BT_LEDS)
 	struct led_trigger	*power_led;
 #endif
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 38063bf1fdc5..c67e6d40c97d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2503,6 +2503,7 @@  struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
 	INIT_LIST_HEAD(&hdev->conn_hash.list);
 	INIT_LIST_HEAD(&hdev->adv_instances);
 	INIT_LIST_HEAD(&hdev->blocked_keys);
+	INIT_LIST_HEAD(&hdev->monitored_devices);
 
 	INIT_LIST_HEAD(&hdev->local_codecs);
 	INIT_WORK(&hdev->rx_work, hci_rx_work);
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 6a943634b31a..b4544b1acf8f 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -80,6 +80,14 @@  struct msft_rp_le_set_advertisement_filter_enable {
 	__u8 sub_opcode;
 } __packed;
 
+#define MSFT_EV_LE_MONITOR_DEVICE	0x02
+struct msft_ev_le_monitor_device {
+	__u8     addr_type;
+	bdaddr_t bdaddr;
+	__u8     monitor_handle;
+	__u8     monitor_state;
+} __packed;
+
 struct msft_monitor_advertisement_handle_data {
 	__u8  msft_handle;
 	__u16 mgmt_handle;
@@ -294,6 +302,15 @@  static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
 		if (monitor && !msft->suspending)
 			hci_free_adv_monitor(hdev, monitor);
 
+		/* Clear any monitored devices by this Adv Monitor */
+		list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices,
+					 list) {
+			if (dev->handle == handle_data->mgmt_handle) {
+				list_del(&dev->list);
+				kfree(dev);
+			}
+		}
+
 		list_del(&handle_data->list);
 		kfree(handle_data);
 	}
@@ -538,6 +555,7 @@  void msft_do_close(struct hci_dev *hdev)
 	struct msft_data *msft = hdev->msft_data;
 	struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
 	struct adv_monitor *monitor;
+	struct monitored_device *dev, *tmp_dev;
 
 	if (!msft)
 		return;
@@ -557,6 +575,16 @@  void msft_do_close(struct hci_dev *hdev)
 		list_del(&handle_data->list);
 		kfree(handle_data);
 	}
+
+	hci_dev_lock(hdev);
+
+	/* Clear any devices that are being monitored */
+	list_for_each_entry_safe(dev, tmp_dev, &hdev->monitored_devices, list) {
+		list_del(&dev->list);
+		kfree(dev);
+	}
+
+	hci_dev_unlock(hdev);
 }
 
 void msft_register(struct hci_dev *hdev)
@@ -590,10 +618,103 @@  void msft_unregister(struct hci_dev *hdev)
 	kfree(msft);
 }
 
+/* This function requires the caller holds hdev->lock */
+static void msft_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr,
+			      __u8 addr_type, __u16 mgmt_handle)
+{
+	struct monitored_device *dev;
+
+	dev = kmalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		bt_dev_err(hdev, "MSFT vendor event %u: no memory",
+			   MSFT_EV_LE_MONITOR_DEVICE);
+		return;
+	}
+
+	bacpy(&dev->bdaddr, bdaddr);
+	dev->addr_type = addr_type;
+	dev->handle = mgmt_handle;
+	dev->notified = false;
+
+	INIT_LIST_HEAD(&dev->list);
+	list_add(&dev->list, &hdev->monitored_devices);
+}
+
+/* This function requires the caller holds hdev->lock */
+static void msft_device_lost(struct hci_dev *hdev, bdaddr_t *bdaddr,
+			     __u8 addr_type, __u16 mgmt_handle)
+{
+	struct monitored_device *dev, *tmp;
+
+	list_for_each_entry_safe(dev, tmp, &hdev->monitored_devices, list) {
+		if (dev->handle == mgmt_handle) {
+			list_del(&dev->list);
+			kfree(dev);
+
+			break;
+		}
+	}
+}
+
+static void *msft_skb_pull(struct hci_dev *hdev, struct sk_buff *skb,
+			   u8 ev, size_t len)
+{
+	void *data;
+
+	data = skb_pull_data(skb, len);
+	if (!data)
+		bt_dev_err(hdev, "Malformed MSFT vendor event: 0x%02x", ev);
+
+	return data;
+}
+
+/* This function requires the caller holds hdev->lock */
+static void msft_monitor_device_evt(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct msft_ev_le_monitor_device *ev;
+	struct msft_monitor_advertisement_handle_data *handle_data;
+	u8 addr_type;
+
+	ev = msft_skb_pull(hdev, skb, MSFT_EV_LE_MONITOR_DEVICE, sizeof(*ev));
+	if (!ev)
+		return;
+
+	bt_dev_dbg(hdev,
+		   "MSFT vendor event 0x%02x: handle 0x%04x state %d addr %pMR",
+		   MSFT_EV_LE_MONITOR_DEVICE, ev->monitor_handle,
+		   ev->monitor_state, &ev->bdaddr);
+
+	handle_data = msft_find_handle_data(hdev, ev->monitor_handle, false);
+
+	switch (ev->addr_type) {
+	case ADDR_LE_DEV_PUBLIC:
+		addr_type = BDADDR_LE_PUBLIC;
+		break;
+
+	case ADDR_LE_DEV_RANDOM:
+		addr_type = BDADDR_LE_RANDOM;
+		break;
+
+	default:
+		bt_dev_err(hdev,
+			   "MSFT vendor event 0x%02x: unknown addr type 0x%02x",
+			   MSFT_EV_LE_MONITOR_DEVICE, ev->addr_type);
+		return;
+	}
+
+	if (ev->monitor_state)
+		msft_device_found(hdev, &ev->bdaddr, addr_type,
+				  handle_data->mgmt_handle);
+	else
+		msft_device_lost(hdev, &ev->bdaddr, addr_type,
+				 handle_data->mgmt_handle);
+}
+
 void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 {
 	struct msft_data *msft = hdev->msft_data;
-	u8 event;
+	u8 *evt_prefix;
+	u8 *evt;
 
 	if (!msft)
 		return;
@@ -602,13 +723,12 @@  void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 	 * matches, and otherwise just return.
 	 */
 	if (msft->evt_prefix_len > 0) {
-		if (skb->len < msft->evt_prefix_len)
+		evt_prefix = msft_skb_pull(hdev, skb, 0, msft->evt_prefix_len);
+		if (!evt_prefix)
 			return;
 
-		if (memcmp(skb->data, msft->evt_prefix, msft->evt_prefix_len))
+		if (memcmp(evt_prefix, msft->evt_prefix, msft->evt_prefix_len))
 			return;
-
-		skb_pull(skb, msft->evt_prefix_len);
 	}
 
 	/* Every event starts at least with an event code and the rest of
@@ -617,10 +737,23 @@  void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 	if (skb->len < 1)
 		return;
 
-	event = *skb->data;
-	skb_pull(skb, 1);
+	hci_dev_lock(hdev);
+
+	evt = msft_skb_pull(hdev, skb, 0, sizeof(*evt));
+	if (!evt)
+		return;
+
+	switch (*evt) {
+	case MSFT_EV_LE_MONITOR_DEVICE:
+		msft_monitor_device_evt(hdev, skb);
+		break;
 
-	bt_dev_dbg(hdev, "MSFT vendor event %u", event);
+	default:
+		bt_dev_dbg(hdev, "MSFT vendor event 0x%02x", *evt);
+		break;
+	}
+
+	hci_dev_unlock(hdev);
 }
 
 __u64 msft_get_features(struct hci_dev *hdev)