[PATCH] sysctl: Fix division by zero in percpu_pagelist_fraction handler

[Oleg Drokin]

Hello!

On Jun 2, 2014, at 9:40 PM, David Rientjes wrote:
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 5dba293..91d0265 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -5854,7 +5854,7 @@ int percpu_pagelist_fraction_sysctl_handler(ctl_table *table, int write,
>> 	int ret;
>> 
>> 	ret = proc_dointvec_minmax(table, write, buffer, length, ppos);
>> -	if (!write || (ret < 0))
>> +	if (!write || (ret < 0) || !*length)
>> 		return ret;
>> 
>> 	mutex_lock(&pcp_batch_high_lock);
> This hasn't made it to linux-next yet (probably because you didn't cc 
> Andrew Morton, the mm maintainer), but I'm wondering why it's needed.  
> Shouldn't this value always be >= min_percpu_pagelist_fract?
> 
> If there's something going on in proc_dointvec_minmax() that disregards 
> that minimum then we need to fix it rather than the caller.

It's needed because at the very beginning of proc_dointvec_minmax it checks if the length is 0 and if it is, immediately returns success because there's nothing to be done
from it's perspective.
And since percpu_pagelist_fraction is 0 from the very beginning, next step is division by this value.

If length is not 0 on the other hand, then there's some value proc_dointvec_minmax can interpret and the min and max checks happen and everything works correctly.

This is kind of hard to fix in proc_dointvec_minmax because there's no passed in value to check against, I think. Sure, you can also check the passed in pointer to value
to make sure it's within range, but that goes beyond the scope of the function.
You might as well just assign a sane value to percpu_pagelist_fraction, which has an added benefit of when you cat that /proc file, you'll get real value of what the kernel uses instead of 0.

Bye,
    Oleg