Messages in this thread | | | Date | Tue, 16 Oct 2018 17:21:01 +0200 | From | Christian Brauner <> | Subject | Re: [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 > >
| |