Page MenuHomeFreeBSD

Make cached Bluetooth LE host advertise information visible from userland.
ClosedPublic

Authored by takawata on Apr 11 2017, 5:13 PM.

Details

Summary

In read_neighbor_cache sub command of hccontrol, there is no information on address type. And bluetooth LE advertise contains extended inquiry and RSSI.

This diff exports these information for user land and make hccontrol recognize them.

Test Plan

Prepare bluetooth LE device and
%hccontrol le_set_scan_enable enable
% hccontrol read_neighbor_cache
and view result.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

takawata created this revision.Apr 11 2017, 5:13 PM
emax requested changes to this revision.Apr 12 2017, 6:12 PM

can you please add boundary checks? thanks!

sys/netgraph/bluetooth/hci/ng_hci_evnt.c
435 ↗(On Diff #27357)

do we need to make sure that mbuf really has n->extinq_data bytes available? what if actual data size is less than header says?

usr.sbin/bluetooth/hccontrol/node.c
234 ↗(On Diff #27357)

do we need to check elemlen here as well? basically what if elemlen is longer than supplied length?

279 ↗(On Diff #27357)

i think this is off by one, i.e. sizeof() will return 4, but last index is 3. what do you think?

This revision now requires changes to proceed.Apr 12 2017, 6:12 PM
takawata updated this revision to Diff 27466.Apr 15 2017, 3:32 PM
takawata edited edge metadata.

add NULL pointer checks pointed out.

emaste added a subscriber: emaste.Apr 18 2017, 1:26 PM
emax requested changes to this revision.Apr 18 2017, 8:56 PM

please see my comments above. thanks!

sys/netgraph/bluetooth/hci/ng_hci_evnt.c
440 ↗(On Diff #27466)

NG_HCI_M_PULLUP() alerts on result being NULL already. also, i think, we still need to check if mbuf actually has enough data.

448 ↗(On Diff #27466)

same comment as above, i.e. NG_HCI_M_PULLUP() warns, and, please check length to make sure data are present

This revision now requires changes to proceed.Apr 18 2017, 8:56 PM
takawata planned changes to this revision.Apr 20 2017, 2:43 PM
takawata added inline comments.
sys/netgraph/bluetooth/hci/ng_hci_evnt.c
440 ↗(On Diff #27466)

Ok. I'll simply use m_pullup instead.

takawata updated this revision to Diff 27578.Apr 20 2017, 3:06 PM
takawata edited edge metadata.

Use m_pullup instead of NG_HCI_M_PULLUP, if mbuf has enough space, m_pullup will return NULL.

emax accepted this revision.Apr 24 2017, 7:33 PM
This revision is now accepted and ready to land.Apr 24 2017, 7:33 PM
This revision was automatically updated to reflect the committed changes.
ngie added a subscriber: ngie.Apr 27 2017, 6:59 PM

I'm going to nitpick the commit a tad...

head/sys/netgraph/bluetooth/hci/ng_hci_evnt.c
428

Whitespace

434–435

Isn't this the same as n->extinq_size = MIN(length_data, NG_HCI_EXTINQ_MAX);?

439

Whitespace

445

Remove space after char, i.e., event = m_pullup(event, sizeof(char));

447

Whitespace

head/sys/netgraph/bluetooth/hci/ng_hci_main.c
100

Whitespace

head/usr.sbin/bluetooth/hccontrol/node.c
211

Please don't redefine MIN/MAX -- it's available in sys/param.h.

213

The definition format is incorrect:

static int
hci_dump_adv(uint8_t *data, int length)
{
219

style(9): please add whitespace, e.g.,

while (length > 0) {

224

Whitespace

230

Whitespace

232

Whitespace

238

Whitespace

245

Whitespace

290
  • Whitespace
  • Please use nitems(..) instead of sizeof(x) / sizeof(x[0])
306–307

Wrap to a single line?

308

Why not add this to the previous fprintf?