From owner-freebsd-hackers@FreeBSD.ORG Thu Feb 24 16:56:50 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E900F1065694 for ; Thu, 24 Feb 2011 16:56:49 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id 7E69B8FC08 for ; Thu, 24 Feb 2011 16:56:49 +0000 (UTC) Received: by wwb31 with SMTP id 31so957552wwb.31 for ; Thu, 24 Feb 2011 08:56:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=AnYH++niiRg8DqugJgRx59weS7JIt0mgeThUvOLeyjU=; b=MRozaCr16BZ8AMMZZYhmIg3cwtzWfnj++X3z52xn964E5vBrDNsnqISa7N4nQqGArm CRUzU1Fi+LfvRLkoOgngyXHt8gtHdhPuB9AHUxkjzVumybx9OYuGFdL7X8x6OrVcr/+g GgtGjYCeOc37I7fZpsMnu9FTyZnusN3KGOivI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=kvO0GOUj4kG1hRVF/3V61SlGH/Yx4HKtREAq2YgUPaW2WbSxbNUr8b6hh5LmjEzblU KTSiYQZwKcj8GumUZzbDj2jbvcLClQ+LOqR5V8Sqvj5OHBFoW19ou4IaX8SIOabkqa3s HynUmWP5pREMV9+iL2xITKF7zFVJFFlYToMz0= MIME-Version: 1.0 Received: by 10.216.145.135 with SMTP id p7mr6044269wej.38.1298566607576; Thu, 24 Feb 2011 08:56:47 -0800 (PST) Received: by 10.216.86.200 with HTTP; Thu, 24 Feb 2011 08:56:47 -0800 (PST) In-Reply-To: References: Date: Thu, 24 Feb 2011 08:56:47 -0800 Message-ID: From: Matthew Fleming To: Dmitry Krivenok Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org Subject: Re: mtx_init/lock_init and uninitialized struct mtx X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Feb 2011 16:56:50 -0000 On Thu, Feb 24, 2011 at 7:47 AM, Dmitry Krivenok 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