From owner-freebsd-fs@FreeBSD.ORG Sat Jan 25 00:38:37 2014 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 20D0BE6D; Sat, 25 Jan 2014 00:38:37 +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 C7670194B; Sat, 25 Jan 2014 00:38:36 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: X-IronPort-AV: E=Sophos;i="4.95,716,1384318800"; d="scan'208";a="90887200" Received: from muskoka.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.222]) by esa-jnhn.mail.uoguelph.ca with ESMTP; 24 Jan 2014 19:38:35 -0500 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id A287FB3F46; Fri, 24 Jan 2014 19:38:35 -0500 (EST) Date: Fri, 24 Jan 2014 19:38:35 -0500 (EST) From: Rick Macklem To: Roman Divacky Message-ID: <1577222508.16050114.1390610315654.JavaMail.root@uoguelph.ca> In-Reply-To: <20140124184141.GA19458@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.202] X-Mailer: Zimbra 7.2.1_GA_2790 (ZimbraWebClient - FF3.0 (Win)/7.2.1_GA_2790) Cc: 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: Sat, 25 Jan 2014 00:38:37 -0000 Roman Divacky wrote: > Hi, > > In nfs_nfsdstate.c:nfsrv_lockctrl() we call > > getlckret = nfsrv_getlockfh(vp, new_stp->ls_flags, NULL, &nfh, p); > > then in nfsrv_getlockfh() we, based on the value of flags, might > dereference the NULL pointer: > > > nfsrv_getlockfh(vnode_t vp, u_short flags, > struct nfslockfile **new_lfpp, fhandle_t *nfhp, NFSPROC_T *p) > > > if (flags & NFSLCK_OPEN) { > new_lfp = *new_lfpp; > fhp = &new_lfp->lf_fh; > > I took a look and don't see a problem. NFSLCK_OPEN is only set for Opens { nfsrvd_open() } and it never calls nfsrv_lockctrl(). nfsrv_lockctrl() is always called with other flags in new_stp->ls_flags set, but never NFSLCK_OPEN, because nfsrv_lockctrl() is handling byte range lock cases after a file has been opened { which means the "else" case for this "if" always applies. You could add a KASSERT() like: KASSERT((new_stp->ls_flags & NFSLCK_OPEN) == 0, ("nfsrv_lockctrl: calling nfsrv_getlockfh with NFSLCK_OPEN")); just before the nfsrv_getlockfh() call if you'd like, but I'd be very surprised if this ever happens. It wouldn't make sense to call nfsrv_lockctrl() with NFSLCK_OPEN and I can't see anywhere it the code that it would do this. (And I've never seen a report of a crash that this would have caused.) > I am not sure what the right fix is. Or if it's even possible to hit > (but I think it is). Anyway the compiler currently generates > a trap instruction (ud2 on x86) in this code. It's the only trap > in GENERIC btw. > Sorry, I'm not a compiler guy, so I don't know why a compiler would generate a trap instruction, but since new_lfpp is never NULL when this is executed, I don't see a problem. If others feel that this needs to be re-coded, please let me know what you think the code should look like? (A test for non-NULL with a panic() before it is used?) Is a trap instruction that never gets executed a problem? rick > Would be lovely to fix this. > > Roman > > P.S. CC me on your replies as I am not subscribed to the list. > > _______________________________________________ > freebsd-fs@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-fs > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org" >