ng_hci: build the devd notification in a stack buffer

[seuros]

Summary

sbuf_new_auto() sleeps, and ng_hci_default_rcvmsg() can run under the
raw HCI socket pcb mutex held across NG_SEND_MSG_PATH().

Reported by: WITNESS
Fixes: ad91d47db306

[imp]

sys/netgraph/bluetooth/hci/ng_hci_main.c

132

So this is 64 bytes... It's likely OK since we're in a leaf that's likely not in a deep call stack. But that's right on the edge and why we did the allocation at first, IIRC.

[adrian]

sys/netgraph/bluetooth/hci/ng_hci_main.c

132

Yeah, I'm trying to figure out how we'd use sbuf inside whatever these locked paths are. Surely this isn't the only place we'd like to use sbuf for things like this.

seuros@, would you mind posting the witness backtrace? Let's find where the lock(s) are being held here.

[seuros]

sys/netgraph/bluetooth/hci/ng_hci_main.c

132

WITNESS fires 3 times per event

uma_zalloc_debug: zone "malloc-16" with the following non-sleepable locks held:
exclusive sleep mutex btsocks_hci_raw_pcb_mtx (btsocks_hci_raw_pcb_mtx) r = 0 (0xfffff801ba3d15e0) locked @ /usr/src/sys/netgraph/bluetooth/socket/ng_btsocket_hci_raw.c:1045
stack backtrace:
#0 0xffffffff80cc52cc at witness_debugger+0x6c
#1 0xffffffff80cc6bcc at witness_warn+0x4bc
#2 0xffffffff8103eaff at uma_zalloc_debug+0x3f
#3 0xffffffff8103e647 at uma_zalloc_arg+0x27
#4 0xffffffff80c1613f at malloc+0x8f
#5 0xffffffff80cabcf4 at sbuf_new+0x154
#6 0xffffffff83841598 at ng_hci_default_rcvmsg+0x4c8
#7 0xffffffff83837e11 at ng_apply_item+0x3d1
#8 0xffffffff83837882 at ng_snd_item+0x2f2
#9 0xffffffff8385fa32 at ng_btsocket_hci_raw_control+0x672
#10 0xffffffff80cd9090 at soo_ioctl+0x450
#11 0xffffffff80ccc27d at kern_ioctl+0x2bd
#12 0xffffffff80ccbf5f at sys_ioctl+0x12f
#13 0xffffffff811db3dc at amd64_syscall+0x17c
#14 0xffffffff811a879b at fast_syscall_common+0xf8

[glebius]

This thread is sleepable in theory, cause it is userland ioctl() on a netgraph socket. So you could indeed maneuver around the lock. But there is no strong rule in netgraph that would say certain kind of a message shall be received only in a sleepable thread. A message can be received from an other node and thread context would an interrupt. Ideally such rules probably shall exists, e.g. some messages shall come only from userland. But we aren't there yet. Given that, I would say patch is good enough for now.

[adrian]

glebius says it's okay so I say it's okay :-)

(what we could do to avoid the stack allocation is to pre-allocate the sbuf generated string component and attach that to the node softc so we can just dup/copy/reference it as needed and just avoid the sbuf-per-notification-in-potentially-sleeping-context problem, but we can tackle that later.)