linux-kernel - Re: [PATCH 4.4 014/168] sysctl: handle overflow for file-max

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]

Message-ID: <CAHrFyr6w3c-hJTYPUP8uxcrg4d_2PzxRP4HAh_-zdcF5igfGWg@mail.gmail.com>
Date:   Wed, 24 Apr 2019 23:50:22 +0200
From:   Christian Brauner <christian@...uner.io>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        stable@...r.kernel.org, Kees Cook <keescook@...omium.org>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.4 014/168] sysctl: handle overflow for file-max

On Wed, Apr 24, 2019 at 7:16 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> [ Upstream commit 32a5ad9c22852e6bd9e74bdec5934ef9d1480bc5 ]
>
> Currently, when writing
>
>   echo 18446744073709551616 > /proc/sys/fs/file-max
>
> /proc/sys/fs/file-max will overflow and be set to 0.  That quickly
> crashes the system.
>
> This commit sets the max and min value for file-max.  The max value is
> set to long int.  Any higher value cannot currently be used as the
> percpu counters are long ints and not unsigned integers.
>
> Note that the file-max value is ultimately parsed via
> __do_proc_doulongvec_minmax().  This function does not report error when
> min or max are exceeded.  Which means if a value largen that long int is
> written userspace will not receive an error instead the old value will be
> kept.  There is an argument to be made that this should be changed and
> __do_proc_doulongvec_minmax() should return an error when a dedicated min
> or max value are exceeded.  However this has the potential to break
> userspace so let's defer this to an RFC patch.
>
> Link: http://lkml.kernel.org/r/20190107222700.15954-3-christian@brauner.io
> Signed-off-by: Christian Brauner <christian@...uner.io>
> Acked-by: Kees Cook <keescook@...omium.org>
> Cc: Alexey Dobriyan <adobriyan@...il.com>
> Cc: Al Viro <viro@...iv.linux.org.uk>
> Cc: Dominik Brodowski <linux@...inikbrodowski.net>
> Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
> Cc: Joe Lawrence <joe.lawrence@...hat.com>
> Cc: Luis Chamberlain <mcgrof@...nel.org>
> Cc: Waiman Long <longman@...hat.com>
> [christian@...uner.io: v4]
>   Link: http://lkml.kernel.org/r/20190210203943.8227-3-christian@brauner.io
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>

Hey Greg,

Just an heads-up. This patch triggered a KASAN warning and Will has
sent a fix for that
which is already in master.
So if you backport this patch you likely also want to backport

9002b21465fa4d829edfc94a5a441005cffaa972

(See  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9002b21465fa4d829edfc94a5a441005cffaa972
)

Seems we missed a Cc: for stable in there. Sorry about that.
Thanks!
Christian

> ---
>  kernel/sysctl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index beadcf83ceba..2f98b11477b8 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -126,6 +126,7 @@ static int __maybe_unused one = 1;
>  static int __maybe_unused two = 2;
>  static int __maybe_unused four = 4;
>  static unsigned long one_ul = 1;
> +static unsigned long long_max = LONG_MAX;
>  static int one_hundred = 100;
>  #ifdef CONFIG_PRINTK
>  static int ten_thousand = 10000;
> @@ -1603,6 +1604,8 @@ static struct ctl_table fs_table[] = {
>                 .maxlen         = sizeof(files_stat.max_files),
>                 .mode           = 0644,
>                 .proc_handler   = proc_doulongvec_minmax,
> +               .extra1         = &zero,
> +               .extra2         = &long_max,
>         },
>         {
>                 .procname       = "nr_open",
> --
> 2.19.1
>
>
>