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

From: Christian Biere <christianbiere@gmx.de>
To: "Jared D. McNeill" <jmcneill@invisible.ca>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/32563: audioctl of death
Date: Tue, 18 Apr 2006 18:03:09 +0200

 Jared D. McNeill wrote:
 > The following patch isn't perfect (we can't rely on having a giant lock 
 > for all of MI audio), but it should at least show the concept.
 > 
 > I still need to figure out where to keep this simple_lock on a 
 > per-instance basis; the audio filters don't have any knowledge of MI audio 
 > or the hw driver, but they need to be locked to prevent changes to the 
 > filter list on the fly.
 > 
 > I'll have another look at this tonight.
 
 I updated my sources before trying to apply your patch. After updating
 the patch couldn't be applied. I've adapted the patch manually. As
 far as I can see, your patch fixes the problem. I cannot trigger the
 panic anymore.
 
 Thanks,
 Christian
 
 
 Index: audio.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/audio.c,v
 retrieving revision 1.202
 diff -u -r1.202 audio.c
 --- audio.c	28 Mar 2006 17:38:29 -0000	1.202
 +++ audio.c	18 Apr 2006 15:48:38 -0000
 @@ -81,6 +81,7 @@
  #include <sys/conf.h>
  #include <sys/audioio.h>
  #include <sys/device.h>
 +#include <sys/lock.h>
  
  #include <dev/audio_if.h>
  #include <dev/audiovar.h>
 @@ -103,6 +104,9 @@
  
  int	audio_blk_ms = AUDIO_BLK_MS;
  
 +/* XXX: jmcneill */
 +struct simplelock audio_filtlock = SIMPLELOCK_INITIALIZER;
 +
  int	audiosetinfo(struct audio_softc *, struct audio_info *);
  int	audiogetinfo(struct audio_softc *, struct audio_info *);
  
 @@ -656,7 +660,9 @@
  	audio_stream_t ps[AUDIO_MAX_FILTERS];
  	const audio_params_t *from_param;
  	audio_params_t *to_param;
 -	int i, n;
 +	int i, n, error = 0;
 +
 +	simple_lock(&audio_filtlock);
  
  	memset(pf, 0, sizeof(pf));
  	memset(ps, 0, sizeof(ps));
 @@ -681,7 +687,8 @@
  				pf[i]->dtor(pf[i]);
  			audio_stream_dtor(&ps[i]);
  		}
 -		return EINVAL;
 +		error = EINVAL;
 +		goto cleanup;
  	}
  
  	audio_destruct_pfilters(sc);
 @@ -710,7 +717,10 @@
  	}
  	audio_print_params("[HW]", &sc->sc_pr.s.param);
  #endif /* AUDIO_DEBUG */
 -	return 0;
 +
 +cleanup:
 +	simple_unlock(&audio_filtlock);
 +	return error;
  }
  
  static int
 @@ -788,12 +798,16 @@
  {
  	int i;
  
 +	simple_lock(&audio_filtlock);
 +
  	for (i = 0; i < sc->sc_npfilters; i++) {
  		sc->sc_pfilters[i]->dtor(sc->sc_pfilters[i]);
  		sc->sc_pfilters[i] = NULL;
  		audio_stream_dtor(&sc->sc_pstreams[i]);
  	}
  	sc->sc_npfilters = 0;
 +
 +	simple_unlock(&audio_filtlock);
  }
  
  static void
 @@ -845,6 +859,8 @@
  			  const audio_params_t *param)
  {
  
 +	simple_lock(&audio_filtlock);
 +
  	if (list->req_size >= AUDIO_MAX_FILTERS) {
  		printf("%s: increase AUDIO_MAX_FILTERS in sys/dev/audio_if.h\n",
  		       __func__);
 @@ -853,6 +869,8 @@
  	list->filters[list->req_size].factory = factory;
  	list->filters[list->req_size].param = *param;
  	list->req_size++;
 +
 +	simple_unlock(&audio_filtlock);
  }
  
  static void
 @@ -860,6 +878,8 @@
  		       stream_filter_factory_t factory,
  		       const audio_params_t *param)
  {
 +	simple_lock(&audio_filtlock);
 +
  	if (i < 0 || i >= AUDIO_MAX_FILTERS) {
  		printf("%s: invalid index: %d\n", __func__, i);
  		return;
 @@ -868,6 +888,8 @@
  	list->filters[i].param = *param;
  	if (list->req_size <= i)
  		list->req_size = i + 1;
 +
 +	simple_unlock(&audio_filtlock);
  }
  
  static void
 @@ -876,6 +898,8 @@
  			   const audio_params_t *param)
  {
  
 +	simple_lock(&audio_filtlock);
 +
  	if (list->req_size >= AUDIO_MAX_FILTERS) {
  		printf("%s: increase AUDIO_MAX_FILTERS in sys/dev/audio_if.h\n",
  		       __func__);
 @@ -886,6 +910,8 @@
  	list->filters[0].factory = factory;
  	list->filters[0].param = *param;
  	list->req_size++;
 +
 +	simple_unlock(&audio_filtlock);
  }
  
  int
 @@ -1858,6 +1884,7 @@
  	 * setup filter pipeline
  	 */
  	uio_fetcher_ctor(&ufetcher, uio, cb->usedhigh);
 +
  	if (sc->sc_npfilters > 0) {
  		fetcher = &sc->sc_pfilters[sc->sc_npfilters - 1]->base;
  	} else {
 @@ -1897,12 +1924,22 @@
  		 * work with a temporary audio_stream_t to narrow
  		 * splaudio() enclosure
  		 */
 +
 +		simple_lock(&audio_filtlock);
  		if (sc->sc_npfilters > 0) {
  			filter = sc->sc_pfilters[0];
  			filter->set_fetcher(filter, &ufetcher.base);
  		}
  		cc = stream.end - stream.start;
 +		if (sc->sc_npfilters > 0) {
 +			fetcher = &sc->sc_pfilters[sc->sc_npfilters - 1]->base;
 +		} else {
 +			fetcher = &ufetcher.base;
 +		}
 +
  		error = fetcher->fetch_to(fetcher, &stream, cc);
 +		simple_unlock(&audio_filtlock);
 +
  		s = splaudio();
  		if (sc->sc_npfilters > 0) {
  			cb->fstamp += audio_stream_get_used(sc->sc_pustream)
 @@ -1947,6 +1984,7 @@
  			audio_fill_silence(&cb->s.param, einp, cc);
  		}
  	}
 +
  	return error;
  }