Messages in this thread | | | Date | Fri, 12 Oct 2018 00:29:35 +0200 | Subject | Re: overflow on proc_nr_files | From | Christian Brauner <> |
| |
On October 11, 2018 7:18:15 PM GMT+02:00, Kees Cook <keescook@chromium.org> wrote: >On Thu, Oct 11, 2018 at 7:10 AM, Christian Brauner ><christian@brauner.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?
Yeah, either that or (I need to look at the code again though if that's possible.) return EOVERFLOW but provide a way (macro) for userspace to know what the maximum value is that this can be set to.
I can send a patch and we can take it from there.
| |