Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 3 Sep 2007 15:07:05 -0400
From:      "Constantine A. Murenin" <cnst@FreeBSD.org>
To:        "Robert Watson" <rwatson@freebsd.org>
Cc:        cvs-src@freebsd.org, Shteryana Shopova <syrinx@freebsd.org>, src-committers@freebsd.org, cvs-all@freebsd.org, "Constantine A. Murenin" <cnst@freebsd.org>
Subject:   Re: cvs commit: src/sys/kern kern_sysctl.c
Message-ID:  <f34ca13c0709031207s371af08am17edf1b880feab9c@mail.gmail.com>
In-Reply-To: <200709020959.l829xYGo077991@repoman.freebsd.org>
References:  <200709020959.l829xYGo077991@repoman.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 02/09/07, Robert Watson <rwatson@freebsd.org> wrote:
> rwatson     2007-09-02 09:59:33 UTC
>
>   FreeBSD src repository
>
>   Modified files:
>     sys/kern             kern_sysctl.c
>   Log:
>   In userland_sysctl(), call useracc() with the actual newlen value to be
>   used, rather than the one passed via 'req', which may not reflect a
>   rewrite.  This call to useracc() is redundant to validation performed by
>   later copyin()/copyout() calls, so there isn't a security issue here,

Yes, the above is correct.  E.g. down the line in
sysctl_handle_string(9), the SYSCTL_IN() macro is used to call
sysctl_new_user(9), which uses copyin(9).


>   but this could technically lead to excessive validation of addresses if
>   the length in newlen is shorter than req.newlen.

No, this is actually not the case -- 'newlen' can never be less than
'req.newlen', because both are of 'size_t' type, and the 'req'
structure is bzero'ed at the beginning of userland_sysctl(). Hence,
before this fix, useracc(9) in question was called with a zero as its
'len' parameter.


I think it is also noteworthy to mention that this a 12-year-old bug.
It was introduced in 1995 (kern_sysctl.c#rev1.38), although this line
was touched slightly in 1999 (kern_sysctl.c#rev1.91).


Best regards,
Constantine.


>
>   Approved by:    re (kensmith)
>   Reviewed by:    jhb
>   Submitted by:   Constantine A. Murenin <cnst+freebsd@bugmail.mojo.ru>
>   Sponsored by:   Google Summer of Code 2007
>
>   Revision  Changes    Path
>   1.177     +1 -1      src/sys/kern/kern_sysctl.c



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f34ca13c0709031207s371af08am17edf1b880feab9c>