Re: kern/56988 (Bluetooth stack initializes bt_lock too late)

[Iain Hibbert <plunky%ogmig.net@localhost>]

The following reply was made to PR kern/56988; it has been noted by GNATS.

From: Iain Hibbert <plunky%ogmig.net@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/56988 (Bluetooth stack initializes bt_lock too late)
Date: Sun, 1 Jan 2023 13:24:39 +0000 (GMT)

 Hi
 
 I agree this is a bit grody, as the Bluetooth stack is not a module
 
 I guess pcmcia devices could also be affected (bt3c and btbc) though I no 
 longer have PCMCIA slots available
 
 devices under dev/bluetooth are pseudo and attached via the stack so 
 nothing there can access this before domain init.
 
 It seems that perhaps the domain init routines should be called before the 
 interfaces attach, but the other uses of .dom_init don't seem to have 
 actual devices attaching to the base of the domain and it hasn't been an 
 issue for net/ as ifinit1() is listed especially in main() before 
 configure gets called.
 
 so, simpler solution could be to wrap the alloc in RUN_ONCE and also call 
 bt_init() from hci_attach_pcb() to ensure the mutex is ready; this avoids 
 entangling with module subsystem, does that sound ok?
 
 iain
 
 Index: bluetooth.h
 ===================================================================
 RCS file: /cvsroot/src/sys/netbt/bluetooth.h,v
 retrieving revision 1.12
 diff -u -p -r1.12 bluetooth.h
 --- bluetooth.h	18 May 2014 14:46:16 -0000	1.12
 +++ bluetooth.h	1 Jan 2023 13:01:36 -0000
 @@ -128,6 +128,7 @@ extern const struct pr_usrreqs l2cap_usr
  extern const struct pr_usrreqs rfcomm_usrreqs;
  
  extern kmutex_t *bt_lock;
 +void bt_init(void);
  
  /*
   * Debugging stuff
 Index: bt_proto.c
 ===================================================================
 RCS file: /cvsroot/src/sys/netbt/bt_proto.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 bt_proto.c
 --- bt_proto.c	21 Jan 2016 15:41:30 -0000	1.16
 +++ bt_proto.c	1 Jan 2023 13:01:36 -0000
 @@ -36,6 +36,7 @@ __KERNEL_RCSID(0, "$NetBSD: bt_proto.c,v
  #include <sys/param.h>
  #include <sys/domain.h>
  #include <sys/kernel.h>
 +#include <sys/once.h>
  #include <sys/protosw.h>
  #include <sys/socket.h>
  #include <sys/systm.h>
 @@ -50,8 +51,6 @@ __KERNEL_RCSID(0, "$NetBSD: bt_proto.c,v
  
  DOMAIN_DEFINE(btdomain);	/* forward declare and add to link set */
  
 -static void	bt_init(void);
 -
  PR_WRAP_CTLOUTPUT(hci_ctloutput)
  PR_WRAP_CTLOUTPUT(sco_ctloutput)
  PR_WRAP_CTLOUTPUT(l2cap_ctloutput)
 @@ -110,9 +109,19 @@ struct domain btdomain = {
  
  kmutex_t *bt_lock;
  
 -static void
 -bt_init(void)
 +static int
 +bt_init_once(void)
  {
  
  	bt_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
 +
 +	return 0;
 +}
 +
 +void
 +bt_init(void)
 +{
 +	static ONCE_DECL(control);
 +
 +	RUN_ONCE(&control, bt_init_once);	
  }
 Index: hci_unit.c
 ===================================================================
 RCS file: /cvsroot/src/sys/netbt/hci_unit.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 hci_unit.c
 --- hci_unit.c	7 Aug 2021 16:19:18 -0000	1.16
 +++ hci_unit.c	1 Jan 2023 13:01:36 -0000
 @@ -93,6 +93,9 @@ hci_attach_pcb(const struct hci_if *hci_
  	KASSERT(hci_if->output_sco != NULL);
  	KASSERT(hci_if->get_stats != NULL);
  
 +	/* we can be reached via autoconf before the stack is initialized */
 +	bt_init();
 +
  	unit = malloc(sizeof(struct hci_unit), M_BLUETOOTH, M_ZERO | M_WAITOK);
  	KASSERT(unit != NULL);