[v2,0/3] sysctl: fix range-checking in do_proc_dointvec_minmax_conv()

Message

Zev Weiss Feb. 7, 2019, 12:34 p.m. UTC

Hello,

After being left with an unusable system after a typo executing
something like 'echo $((1<<24)) > /proc/sys/vm/max_map_count', I found
that do_proc_dointvec_minmax_conv() was missing a check to ensure that
the converted value actually fits in an int.

The first of the following patches enhances the sysctl selftest such
that it detects this problem; the second provides a minimal fix
(suitable for -stable) such that the selftest passes.  The third patch
then performs a more thorough refactoring to eliminate the code
duplication that led to the bug in the first place (maintaining the
passing status of the selftest).


Changes in v2:
 - Rearranged selftest to also test negative values and provide more
   info in comments
 - Added intermediate patch as a minimal fix for -stable without the
   refactoring


Thanks,
Zev Weiss

Comments

Luis Chamberlain Feb. 7, 2019, 3:51 p.m. UTC | #1

On Thu, Feb 07, 2019 at 06:34:23AM -0600, Zev Weiss wrote:
> Hello,
> 
> After being left with an unusable system after a typo executing
> something like 'echo $((1<<24)) > /proc/sys/vm/max_map_count', I found
> that do_proc_dointvec_minmax_conv() was missing a check to ensure that
> the converted value actually fits in an int.
> 
> The first of the following patches enhances the sysctl selftest such
> that it detects this problem; the second provides a minimal fix
> (suitable for -stable) such that the selftest passes.  The third patch
> then performs a more thorough refactoring to eliminate the code
> duplication that led to the bug in the first place (maintaining the
> passing status of the selftest).
> 
> 
> Changes in v2:
>  - Rearranged selftest to also test negative values and provide more
>    info in comments
>  - Added intermediate patch as a minimal fix for -stable without the
>    refactoring

Thanks! For some reason I got all except the last patch, patch #3.
Can you bounce me and others a copy?

  Luis

Zev Weiss Feb. 7, 2019, 4:54 p.m. UTC | #2

On Thu, Feb 07, 2019 at 09:51:44AM CST, Luis Chamberlain wrote:
>On Thu, Feb 07, 2019 at 06:34:23AM -0600, Zev Weiss wrote:
>> Hello,
>>
>> After being left with an unusable system after a typo executing
>> something like 'echo $((1<<24)) > /proc/sys/vm/max_map_count', I found
>> that do_proc_dointvec_minmax_conv() was missing a check to ensure that
>> the converted value actually fits in an int.
>>
>> The first of the following patches enhances the sysctl selftest such
>> that it detects this problem; the second provides a minimal fix
>> (suitable for -stable) such that the selftest passes.  The third patch
>> then performs a more thorough refactoring to eliminate the code
>> duplication that led to the bug in the first place (maintaining the
>> passing status of the selftest).
>>
>>
>> Changes in v2:
>>  - Rearranged selftest to also test negative values and provide more
>>    info in comments
>>  - Added intermediate patch as a minimal fix for -stable without the
>>    refactoring
>
>Thanks! For some reason I got all except the last patch, patch #3.
>Can you bounce me and others a copy?
>
>  Luis

Hmm, odd -- it does seem like each time I use git-send-email I manage to 
find a new way to botch it up, but in this case it *looks* like my 
server logs indicate that one should have been sent properly as far as I 
can tell.  No matter, resent it manually anyway, hopefully it gets 
through this time...(apologies if anyone gets duplicate copies).


Zev