svn commit: r186291 - head/sbin/mount

[David O'Brien]

On Thu, Dec 18, 2008 at 01:13:30PM -0700, M. Warner Losh wrote:
> In message: <200812181844.mBIIikVF049455 at 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 at FreeBSD.org)