Date: Thu, 24 Feb 2011 08:56:47 -0800 From: Matthew Fleming <mdf356@gmail.com> To: Dmitry Krivenok <krivenok.dmitry@gmail.com> Cc: freebsd-hackers@freebsd.org Subject: Re: mtx_init/lock_init and uninitialized struct mtx Message-ID: <AANLkTim3Y69pwv2NGQFK%2Bp0-YZvxDC11_P8AtYddptJ4@mail.gmail.com> In-Reply-To: <AANLkTimGkjDLO7LCgPMKyDGeWTqKZzzFk=bPzkBCfUn6@mail.gmail.com> References: <AANLkTimGkjDLO7LCgPMKyDGeWTqKZzzFk=bPzkBCfUn6@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Feb 24, 2011 at 7:47 AM, Dmitry Krivenok
<krivenok.dmitry@gmail.com> wrote:
> Hello Hackers,
>
> Is it allowed to call mtx_init on a mutex defined as an auto variable
> and not initialized explicitly, i.e.:
We recently ran into this problem at $WORK because we turned on the
deadc0de checking in uma zones for any zone without an explicit
init/fini function, in order to detect more use-after-free scenarios.
It happens that one of the bits in 0xDEADC0DE is the LO_INITIALIZED
bit.
It's a bit of a tough call, since one would like mtx_init(9) and
family to just work on any blob of memory, but one would also like to
catch a programmer error where a lock is re-initialized.
I suppose that for INVARIANTS the kernel can remember the address of
all initialized locks and forget it on lock destroy, and in this way
have a useful assert and also allow lock_init on random storage. This
would also allow for detecting if the memory for a lock was released
but the lock wasn't destroyed.
Sadly, I have just enough time to propose this and not enough to write
a patch at the moment.
Thanks,
matthew
>
> static int foo()
> {
> =A0 struct mtx m; =A0// Uninitialized auto variable, so it's value is und=
efined.
> =A0 mtx_init(&m, "my_mutex", NULL, MTX_DEF);
> =A0 =85
> =A0 // Do something
> =A0 ...
> =A0 mtx_destroy(&m);
> =A0 return 0;
> }
>
> I encountered a problem with such code on a kernel compiled with
> INVARIANTS option.
> The problem is that mtx_init calls lock_init(&m->lock_object) and
> lock_init, in turn, calls:
>
> =A079 =A0 =A0 =A0 =A0 /* Check for double-init and zero object. */
> =A080 =A0 =A0 =A0 =A0 KASSERT(!lock_initalized(lock), ("lock \"%s\" %p al=
ready
> initialized",
> =A081 =A0 =A0 =A0 =A0 =A0 =A0 name, lock));
>
> lock_initialized() just checks that a bit is set in lo_flags field of
> struct lock_object:
>
> 178 #define lock_initalized(lo) =A0 =A0 ((lo)->lo_flags & LO_INITIALIZED)
>
> However, the structure containing this field is never initialized
> (neither in mtx_init nor in lock_init).
> So, assuming that the mutex was defined as auto variable, the content
> of lock_object field of struct mtx
> is also undefined:
>
> =A037 struct mtx {
> =A038 =A0 =A0 =A0 =A0 struct lock_object =A0 =A0 =A0lock_object; =A0 =A0/=
* Common lock
> properties. */
> =A039 =A0 =A0 =A0 =A0 volatile uintptr_t =A0 =A0 =A0mtx_lock; =A0 =A0 =A0=
/* Owner and flags. */
> =A040 };
>
> In some cases, the initial value of lo_flags _may_ have the
> "initialized" bit set and KASSERT will call panic.
>
> Is it user's responsibility to properly (how exactly?) initialize
> struct mtx, e.g.
> memset(&m, '\0', sizeof(struct mtx));
>
> Or should mtx_init() explicitly initialize all fields of struct mtx?
>
> Thanks in advance!
>
> --
> Sincerely yours, Dmitry V. Krivenok
> e-mail: krivenok.dmitry@gmail.com
> skype: krivenok_dmitry
> jabber: krivenok_dmitry@jabber.ru
> icq: 242-526-443
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTim3Y69pwv2NGQFK%2Bp0-YZvxDC11_P8AtYddptJ4>