From owner-svn-src-all@FreeBSD.ORG Tue Dec 23 17:02:51 2008 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B6C601065673; Tue, 23 Dec 2008 17:02:51 +0000 (UTC) (envelope-from obrien@NUXI.org) Received: from dragon.nuxi.org (trang.nuxi.org [74.95.12.85]) by mx1.freebsd.org (Postfix) with ESMTP id 981898FC13; Tue, 23 Dec 2008 17:02:51 +0000 (UTC) (envelope-from obrien@NUXI.org) Received: from dragon.nuxi.org (obrien@localhost [127.0.0.1]) by dragon.nuxi.org (8.14.2/8.14.2) with ESMTP id mBNH2iqa024891; Tue, 23 Dec 2008 09:02:44 -0800 (PST) (envelope-from obrien@dragon.nuxi.org) Received: (from obrien@localhost) by dragon.nuxi.org (8.14.2/8.14.2/Submit) id mBNH2hnL024890; Tue, 23 Dec 2008 09:02:43 -0800 (PST) (envelope-from obrien) Date: Tue, 23 Dec 2008 09:02:43 -0800 From: "David O'Brien" To: "M. Warner Losh" Message-ID: <20081223170243.GA24817@dragon.NUXI.org> References: <200812181844.mBIIikVF049455@svn.freebsd.org> <20081218.131330.63052780.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20081218.131330.63052780.imp@bsdimp.com> X-Operating-System: FreeBSD 8.0-CURRENT User-Agent: Mutt/1.5.16 (2007-06-09) Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r186291 - head/sbin/mount X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: obrien@FreeBSD.org List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Dec 2008 17:02:51 -0000 On Thu, Dec 18, 2008 at 01:13:30PM -0700, M. Warner Losh wrote: > In message: <200812181844.mBIIikVF049455@svn.freebsd.org> > : Log: > : Be a little bit more pestimistic in argument handling - check if .. > : + if (MAX_ARGS <= argc ) > : + errx(1, "Cannot process more than %d mount arguments", > : + MAX_ARGS); > : + > > This is useless. Once you've overflowed the buffer, your stack is > potentially shot, and all kinds of fun can happen downstream from > there. In particular, there's no guarantee that argc isn't corrupted > as well... s/useless/almost useless/. I fully admit its a very thin safety belt (thus the "little bit" in the commit log). The test does catches small overflows, but yes argc could also be easily damaged. I missed pulling out the change to make 'argc' static (thus avoiding the that corruption issue), I also have another patch for you (I'll send privately). > Also, the style of the check is inconsistent with other parts of the > system: > > : + if (argc > MAX_ARGS) > > would be more consistent, and not suffer from the space before the > paren problem as well :) The space before the paren was a typo :-( Actually the only other test against 'argc' is "for (i = 1; i < argc; i++)" so there is consistancy. ;-) -- -- David (obrien@FreeBSD.org)