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

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

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

From: Harold Gutch <logix%foobar.franken.de@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/57649: Uninitialized lock in gus.c
Date: Tue, 10 Oct 2023 12:31:09 +0200

 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