Re: kern/57649: Uninitialized lock in gus.c

[Harold Gutch <logix%foobar.franken.de@localhost>]

On Tue, Oct 10, 2023 at 12:15:02AM +0000, Taylor R Campbell wrote:
> The following reply was made to PR kern/57649; it has been noted by GNATS.
> 
> From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
> To: Harold Gutch <logix%foobar.franken.de@localhost>
> Cc: gnats-bugs%NetBSD.org@localhost
> Subject: Re: kern/57649: Uninitialized lock in gus.c
> Date: Tue, 10 Oct 2023 00:12:40 +0000
> 
>  I think this can't be right.
>  
>  sc->sc_lock and sc->sc_intr_lock appear to be totally unused.  We
>  should just flush them, and delete the bogus

They are used down the line in audio.c.


>  	sc->sc_lock = sc->sc_codec.sc_ad1848.sc_lock;
>  	sc->sc_intr_lock = sc->sc_codec.sc_ad1848.sc_intr_lock;
>  
>  initalization, which has never made sense.
>  
>  The real mutexes, returned by .get_locks = ad1848_get_locks, are
>  already initialized by ad1848_init_locks when gusattach calls that
>  early on.

Ah, indeed, I missed the call to ad1848_init_locks() just a few lines
above...


>  So why are they uninitialized?  Not sure!  Maybe something is trying
>  to use sc->sc_mixer and sc->sc_codec (which are in a union) at the
>  same time and things are stomping all over each other?  Time to printf
>  all the things?

They might not be uninitialized - what would be an easy way to check
this?

This might just be LOCKDEBUG messing up because it keeps tracks of
locks via the variables used to address them - and the two variables
sc->sc_lock and sc->sc_codec.sc_ad1848.sc_lock differ, even if their
value is the same.  A GENERIC kernel with "gus?" on the right base
port but without LOCKDEBUG works for me.  It is "only" LOCKDEBUG that
then makes it panic in audio.c because that thinks sc->sc_lock is
uninitialized.

Is there a reasonable way of "copying" a lock like gus.c wants to do
in the two lines you quoted?


thanks,
  Harold