From owner-freebsd-fs@FreeBSD.ORG Tue Jan 28 22:36:52 2014 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 8317E81C; Tue, 28 Jan 2014 22:36:52 +0000 (UTC) Received: from esa-jnhn.mail.uoguelph.ca (esa-jnhn.mail.uoguelph.ca [131.104.91.44]) by mx1.freebsd.org (Postfix) with ESMTP id 21057110B; Tue, 28 Jan 2014 22:36:51 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: X-IronPort-AV: E=Sophos;i="4.95,738,1384318800"; d="scan'208";a="91787121" Received: from muskoka.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.222]) by esa-jnhn.mail.uoguelph.ca with ESMTP; 28 Jan 2014 17:36:44 -0500 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 3D523B3EEA; Tue, 28 Jan 2014 17:36:44 -0500 (EST) Date: Tue, 28 Jan 2014 17:36:44 -0500 (EST) From: Rick Macklem To: Roman Divacky Message-ID: <2013799744.17924450.1390948604241.JavaMail.root@uoguelph.ca> In-Reply-To: <20140128094045.GB16311@freebsd.org> Subject: Re: BUG: possible NULL pointer dereference in nfs server MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.203] X-Mailer: Zimbra 7.2.1_GA_2790 (ZimbraWebClient - FF3.0 (Win)/7.2.1_GA_2790) Cc: Dimitry Andric , fs@FreeBSD.org X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jan 2014 22:36:52 -0000 Roman Divacky wrote: > > > Yea, so long as it includes a comment that states this is done to > > > work around a stupid compiler bug. > > > > Ugh. > > > > The above has the following bugs: > > - gross style bugs (lines longer than 80 characters) > > - large code to do nothing > > - would be even larger with a comment > > - cannot actually work around any compiler bug involving abort(), > > since it > > has no effect on production kernels where KASSERT() is null. > > > > >> It is present even in your setup :) Just "objdump -d kernel | > > >> grep > > >> ud2" on kernel compiled > > >> by clang. > > >> > > > I actually use gcc, but I believe you. I'll admit I still don't > > > understand > > > why having a trap instruction that never gets executed is a bad > > > thing? > > > > It isn't but trying to link to the noexistent function abort() on > > arches that don't have any trap instruction is a bad thing. > > According > > the above, this occurs on sparc64. It must be a gcc bug, since > > sparc64 > > doesn't have clang. However, I couldn't get gcc on sparc64 to > > generate > > an abort() for *NULL. Similarly on x86 (gcc is hard to test on > > x86, > > since it is broken (not installed) on FreeBSD cluster machines for > > I was testing clang compiled kernel on sparc64. > > The problem is that there's nothing making sure that the NULL pointer > dereference doesnt happen. So if someone happens to call the function > with > wrong flags it will crash. > > Thats why I want to add the KASSERT, to catch possible future cases > when this happens. > > Unfortunately our KASSERT is not an assert so to remove the actual > abort/trap/ud2 I have to remove the flag. > > > Ok? > If it makes clang for sparc64 happy, I suppose so. Another possible coding, which is maybe a little less ugly and might fix the problem is: change if (new_stp->ls_flags & NFSLCK_OPEN) { to if ((new_stp->ls_flags & NFSLCK_OPEN) != 0 && new_lfpp != NULL) { rick > > Roman >