kern/46121: audiomp: locking protocol error

[ad%netbsd.org@localhost]

>Number:         46121
>Category:       kern
>Synopsis:       audiomp: locking protocol error
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Mar 01 10:55:00 +0000 2012
>Originator:     Andrew Doran
>Release:        -current
>Organization:
The NetBSD Project
>Environment:
N/A
>Description:
audio.c has this descriptive block:

    122  * List of hardware interface methods, and which locks are held when 
each
    123  * is called by this module:
    124  *
    125  *      METHOD                  INTR    THREAD  NOTES
    126  *      ----------------------- ------- ------- 
-------------------------
...
    146  *      allocm                  -       -       Called at attach time
    147  *      freem                   -       -       Called at attach time

Lets look at the code.

    381 void
    382 audioattach(device_t parent, device_t self, void *aux)
...
    454         /*
    455          * XXX  Would like to not hold the sc_lock around this whole 
block
    456          * escpially for audio_alloc_ring(), except that the latter 
calls
    457          * ->round_blocksize() which demands the thread lock to be 
taken.
    458          *
    459          * Revisit.
    460          */
    461         mutex_enter(sc->sc_lock);
...
    463                 error = audio_alloc_ring(sc, &sc->sc_pr,
...
    476                         if (sc->sc_pr.s.start != 0)
    477                                 audio_free_ring(sc, &sc->sc_pr);

Further down the chain routines such as this are called:

   1041 static void
   1042 btsco_freem(void *hdl, void *addr, size_t size)
...
   1052                 while (sc->sc_tx_refcnt> 0 && count-- > 0)
   1053                         kpause("drain", false, 1, NULL);

So comment block does not match reality and we call allocm/freem routines with 
lock held. This is a serious problem as:

(a) down the chain we do e.g. kpause(), kmem_alloc() etc.
(b) audio driver "thread" lock is taken from soft interrupt context.
(c) thread lock may be shared with other subsystems.  For instance, in the case 
of bluetooth the thread lock will be bt_lock and this is also used to lock 
bluetooth sockets (so->so_lock == bt_lock). We cannot block with socket locks 
held.
>How-To-Repeat:
Code inspection.
>Fix:
Do not call allocm/freem methods with sc_lock held.  Instead inspect 
allocm/freem methods and calling code path.  Sprinkle synchronization as 
necessary.