linux-kernel - Re: overflow on proc_nr

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: <CAGXu5jK0nbBrJF4Vp3yzhwBgGo3vKh8FVo2ZQJMA+Ph3NaGh+Q@mail.gmail.com>
Date:   Thu, 11 Oct 2018 10:18:15 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Christian Brauner <christian@...uner.io>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        "Eric W . Biederman" <ebiederm@...ssion.com>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Waiman Long <longman@...hat.com>
Subject: Re: overflow on proc_nr_files

On Thu, Oct 11, 2018 at 7:10 AM, Christian Brauner <christian@...uner.io> wrote:
> Hey,
>
> I've just got pinged by Lennart who discovered that you can get your
> system into an unuseable state by writing something that exceeds a s64
> into /proc/sys/fs/file-max. Say,
>
> echo 20000000000000000000 > /proc/sys/fs/file-max
>
> which will trigger an overflow and percpu_counter_read_positive() will
> return 0 and cat /proc/sys/fs/file-max will return 0.
>
> That effectively means you write that number and it succeeds and all is
> well and a few seconds/minutes later your system just dies or gets into
> an unuseable state pretty quickly
>
> I wonder if we shouldn't accept overflows or - if we have no way in this
> codepath to detect them - set it to some pre-defined hard-coded value.
>
> Or maybe this is even a known issue and by design but before I work on a
> patch here I just wanted to check.

There was work done recently to keep proc_dointvec_minmax from
wrapping, but it seems that the problem here is that file-max uses
proc_doulongvec_minmax, so it explicitly thinks it can be larger than
s64. (And max_files itself is unsigned long...)

It looks like the counter is expected to be a long, not unsigned:

static long get_nr_files(void)
{
        return percpu_counter_read_positive(&nr_files);
}

And there are places where this goes weird:

                if (percpu_counter_sum_positive(&nr_files) >=
files_stat.max_files)

etc.

It seems like maybe the sysctl needs to be explicitly capped in
kernel/sysctl.c to S64_MAX?

-Kees

-- 
Kees Cook
Pixel Security