On Tue, Jun 14, 2016 at 01:19:06PM -0700, Andrew Morton wrote:
> On Sat, 11 Jun 2016 03:33:08 +0200 Heinrich Schuchardt <xypron.g...@gmx.de>
> wrote:
>
> > An undetected overflow may occur in do_proc_dointvec_minmax_conv_param.
> >
> > ...
> >
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -2313,7 +2313,17 @@ static int do_proc_dointvec_minmax_conv(bool *negp,
> > unsigned long *lvalp,
> > {
> > struct do_proc_dointvec_minmax_conv_param *param = data;
> > if (write) {
> > - int val = *negp ? -*lvalp : *lvalp;
> > + int val;
> > +
> > + if (*negp) {
> > + if (*lvalp > (unsigned long) INT_MAX + 1)
> > + return -EINVAL;
> > + val = -*lvalp;
> > + } else {
> > + if (*lvalp > (unsigned long) INT_MAX)
> > + return -EINVAL;
> > + val = *lvalp;
> > + }
> > if ((param->min && *param->min > val) ||
> > (param->max && *param->max < val))
> > return -EINVAL;
>
> hm.
>
> What happens if someone does
>
> echo -1 > /proc/foo
>
> expecting to get 0xffffffff? That's a reasonable shorthand, and if we
> change that to spit out EINVAL then people's stuff may break.
I'd go even further, I don't see anymore how it becomes possible
to actually *write* 0xffffffff at all! This function is used by
proc_dointvec_minmax() which is used with extra1=&zero and extra2
not set with some unsigned ints to allow the full range to be
configured (eg: dirty_expire_interval is the first I found by a
quick random look).
So for me this change is bogus.
Willy
