linux-kernel - Re: [PATCH v2 0/9] sysctl: add and fix proper unsigned int support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Feb 2017 12:11:48 -0800
From:   Kees Cook <keescook@...omium.org>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Ingo Molnar <mingo@...nel.org>, Mel Gorman <mgorman@...e.de>,
        Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>,
        Jessica Yu <jeyu@...hat.com>,
        Rusty Russell <rusty@...tcorp.com.au>,
        Steven Whitehouse <swhiteho@...hat.com>,
        deepa.kernel@...il.com, Matt Fleming <matt@...eblueprint.co.uk>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Borislav Petkov <bp@...e.de>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>, shuah@...nel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Guenter Roeck <linux@...ck-us.net>,
        linux-kselftest@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 0/9] sysctl: add and fix proper unsigned int support

On Fri, Feb 10, 2017 at 4:36 PM, Luis R. Rodriguez <mcgrof@...nel.org> wrote:
> On this v2 I've taken Alexey's recommendation and looked at array users
> of the proc sysctl interface which complicate the interfece to see if
> we can instead just simplify the unsigned int implementation. I could
> not find any clear candidate. As such I've just ripped out array
> support.
>
> Since some future unsigned int proc sysctl users might think there is
> array support I've taken measures to do sanity checks on initialization
> and warn the kernel if such users creep up. To validate this I ended up
> just writing a simple test driver, and extending our tests. In doing this
> I also found a really old issue with sysctl_check_table(), and yet another
> issue with the first incarnation of proc_douintvec().
>
> I hammered on proc_douintvec() as much as I could, and extended tests for
> this to ensure we don't regress should some int users convert over.
>
> I noticed one more issue but I did not fix as I figured it was worth
> discussing: proc_doi*_minmax() handlers have historically allowed users
> to register even if their own data does not match the expressed min/max
> values. When this happens the value is exposed on /proc/sys but reading
> or writing does not work against it. I'm of the opinion that
> sysctl_check_table() should just validate this and bail preventing such
> entries from ever creeping up. The only reason I didn't do this is this
> *could* mean some tables don't get registered in some cases -- I haven't
> done the vetting. If we're fine with this I can add it later.
>
> Luis R. Rodriguez (9):
>   sysctl: fix lax sysctl_check_table() sanity check
>   sysctl: add proper unsigned int support
>   sysctl: add unsigned int range support
>   test_sysctl: add dedicated proc sysctl test driver
>   test_sysctl: add generic script to expand on tests
>   test_sysctl: test against PAGE_SIZE for int
>   test_sysctl: add simple proc_dointvec() case
>   test_sysctl: add simple proc_douintvec() case
>   test_sysctl: test against int proc_dointvec() array support

Please go ahead and add a MAINTAINERS file entry for the two of us
(and Eric if he wants) for sysctl. We poke at it enough that really we
should declare it maintained (as you suggested privately). For now we
should likely still land it all through akpm, though.

-Kees

-- 
Kees Cook
Pixel Security