lkml.org 
[lkml]   [2018]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 2/2] sysctl: handle overflow for file-max
    On Tue, Oct 16, 2018 at 11:13:28AM -0400, Waiman Long wrote:
    > On 10/15/2018 06:55 AM, Christian Brauner wrote:
    > > 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 explicitly caps the value for file-max to ULONG_MAX.
    > >
    > > Note, this isn't technically necessary since proc_get_long() will already
    > > return ULONG_MAX. However, two reason why we still should do this:
    > > 1. it makes it explicit what the upper bound of file-max is instead of
    > > making readers of the code infer it from proc_get_long() themselves
    > > 2. other tunebles than file-max may want to set a lower max value than
    > > ULONG_MAX and we need to enable __do_proc_doulongvec_minmax() to handle
    > > such cases too
    > >
    > > Cc: Kees Cook <keescook@chromium.org>
    > > Signed-off-by: Christian Brauner <christian@brauner.io>
    > > ---
    > > v0->v1:
    > > - if max value is < than ULONG_MAX use max as upper bound
    > > - (Dominik) remove double "the" from commit message
    > > ---
    > > kernel/sysctl.c | 4 ++++
    > > 1 file changed, 4 insertions(+)
    > >
    > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
    > > index 97551eb42946..226d4eaf4b0e 100644
    > > --- a/kernel/sysctl.c
    > > +++ b/kernel/sysctl.c
    > > @@ -127,6 +127,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 ulong_max = ULONG_MAX;
    > > static int one_hundred = 100;
    > > static int one_thousand = 1000;
    > > #ifdef CONFIG_PRINTK
    > > @@ -1696,6 +1697,7 @@ static struct ctl_table fs_table[] = {
    > > .maxlen = sizeof(files_stat.max_files),
    > > .mode = 0644,
    > > .proc_handler = proc_doulongvec_minmax,
    > > + .extra2 = &ulong_max,
    >
    > What is the point of having a maximum value of ULONG_MAX anyway? No
    > value you can put into a ulong type can be bigger than that.

    This is changed in the new code to LONG_MAX. See the full thread for
    context. There's also an additional explantion in the commit message.

    >
    > > },
    > > {
    > > .procname = "nr_open",
    > > @@ -2795,6 +2797,8 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
    > > break;
    > > if (neg)
    > > continue;
    > > + if (max && val > *max)
    > > + val = *max;
    > > val = convmul * val / convdiv;
    > > if ((min && val < *min) || (max && val > *max))
    > > continue;
    >
    > This does introduce a change in behavior. Previously the out-of-bound
    > value is ignored, now it is capped at its maximum. This is a
    > user-visible change.

    Not completely true though. Try

    echo 18446744073709551616 > /proc/sys/fs/file-max

    on a system you find acceptable loosing.
    So this is an acceptable user-visible change I'd say. But I'm open to
    other suggestions.

    >
    > Cheers,
    > Longman
    >
    >

    \
     
     \ /
      Last update: 2018-10-16 17:21    [W:2.885 / U:0.524 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site