lowmem_reserve_ratio_sysctl_handler() ignores the return value of
proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
even for read operations.

Fix two issues:

1. Propagate errors from proc_dointvec_minmax() instead of always
   returning success.  For example, writing non-integer garbage to the
   sysctl now returns an error instead of silently succeeding with
   unchanged values.

2. Only call setup_per_zone_lowmem_reserve() when the sysctl is
   actually written.  Reading /proc/sys/vm/lowmem_reserve_ratio should
   not recompute derived lowmem_reserve[] and totalreserve_pages when
   the inputs did not change, matching min_free_kbytes and
   watermark_scale_factor handlers.

Drop the manual "< 1 -> 0" sanitization loop and set .extra1 =
SYSCTL_ZERO on the ctl_table entry so proc_dointvec_minmax() enforces
the minimum on write; negative values now return -EINVAL instead of
being silently coerced to 0 (suggested by Vlastimil Babka).

Link: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Jianlin Shi <shijianlin11@foxmail.com>
---
Changes in v3:
- Rewrite commit log to focus on the two tangible fixes as suggested by
  Johannes Weiner.
- Code remains unchanged versus v2; retain previously obtained tags.

Changes in v2:
- Add .extra1 = SYSCTL_ZERO to ctl_table entry
- Remove manual sanitization loop; negative writes now return -EINVAL

v1: https://lore.kernel.org/linux-mm/tencent_FFD4F4D728AAE8A8AE0AF277A59854A29A06@qq.com/

 mm/page_alloc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0387d2afd..a7381327d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6683,16 +6683,15 @@ static int sysctl_min_slab_ratio_sysctl_handler(const struct ctl_table *table, i
 static int lowmem_reserve_ratio_sysctl_handler(const struct ctl_table *table,
 		int write, void *buffer, size_t *length, loff_t *ppos)
 {
-	int i;
+	int rc;
 
-	proc_dointvec_minmax(table, write, buffer, length, ppos);
+	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
+	if (rc)
+		return rc;
 
-	for (i = 0; i < MAX_NR_ZONES; i++) {
-		if (sysctl_lowmem_reserve_ratio[i] < 1)
-			sysctl_lowmem_reserve_ratio[i] = 0;
-	}
+	if (write)
+		setup_per_zone_lowmem_reserve();
 
-	setup_per_zone_lowmem_reserve();
 	return 0;
 }
 
@@ -6791,6 +6790,7 @@ static const struct ctl_table page_alloc_sysctl_table[] = {
 		.maxlen		= sizeof(sysctl_lowmem_reserve_ratio),
 		.mode		= 0644,
 		.proc_handler	= lowmem_reserve_ratio_sysctl_handler,
+		.extra1		= SYSCTL_ZERO,
 	},
 #ifdef CONFIG_NUMA
 	{
-- 
2.43.0

On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> wrote:

> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
> proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
> even for read operations.
> 
> Fix two issues:
> 
> 1. Propagate errors from proc_dointvec_minmax() instead of always
>    returning success.  For example, writing non-integer garbage to the
>    sysctl now returns an error instead of silently succeeding with
>    unchanged values.

AI review suggest that this caused a new problem:

	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com

Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
temporary then copy that into sysctl_lowmem_reserve_ratio if all
proc_dointvec_minmax() returns "OK".

But really this is a flaw in proc_dointvec_minmax() isn't it?  It
shouldn't update the table data until all the data has been validated.

+Cc: sysctl maintainers

On 8/1/26 20:44, Andrew Morton wrote:
> On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> wrote:
> 
>> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
>> proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
>> even for read operations.
>> 
>> Fix two issues:
>> 
>> 1. Propagate errors from proc_dointvec_minmax() instead of always
>>    returning success.  For example, writing non-integer garbage to the
>>    sysctl now returns an error instead of silently succeeding with
>>    unchanged values.
> 
> AI review suggest that this caused a new problem:
> 
> 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
> 
> Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> temporary then copy that into sysctl_lowmem_reserve_ratio if all
> proc_dointvec_minmax() returns "OK".

Seeing the v4 [1] it seems easier to keep the current fixup code until
proc_dointvec_minmax() is fixed.

> But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> shouldn't update the table data until all the data has been validated.

I agree. What do the maintainers think?

[1]
https://lore.kernel.org/all/tencent_B9556590D4B65ACF74C06897689A6E39F206@qq.com/

On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
> +Cc: sysctl maintainers
> 
> On 8/1/26 20:44, Andrew Morton wrote:
> > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> wrote:
> > 
> >> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
> >> proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
> >> even for read operations.
> >> 
> >> Fix two issues:
> >> 
> >> 1. Propagate errors from proc_dointvec_minmax() instead of always
> >>    returning success.  For example, writing non-integer garbage to the
> >>    sysctl now returns an error instead of silently succeeding with
> >>    unchanged values.
> > 
> > AI review suggest that this caused a new problem:
> > 
> > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
> > 
> > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> > temporary then copy that into sysctl_lowmem_reserve_ratio if all
> > proc_dointvec_minmax() returns "OK".
> 
> Seeing the v4 [1] it seems easier to keep the current fixup code until
> proc_dointvec_minmax() is fixed.

1. V3 -vs- V4:
  I would prefer V4 as it actually prevents the partial write of the
  vector in case of an error & returns that error back to user space.
  Whereas V3 returns the error back to user space but keeps the partial
  write.

That patterns of using a temp ctltable entry is seldom used but not
unheard of.

> 
> > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> > shouldn't update the table data until all the data has been validated.
> 
> I agree. What do the maintainers think?

I agree. The arrays being changed are not too big so we can easily have
a staging variable that that gets written when all validations are done.
The only "con" that I see for this solution is for when these
proc_handlers get used with temp variables; in these cases we will be
staging an already staging variable.

I have added this to my Todos

Best

-- 

Joel

On Tue, Aug 04, 2026 at 02:20:22PM +0200, Joel Granados wrote:
> On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
> > +Cc: sysctl maintainers
> > 
> > On 8/1/26 20:44, Andrew Morton wrote:
> > > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> wrote:
> > > 
> > >> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
> > >> proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
> > >> even for read operations.
> > >> 
> > >> Fix two issues:
> > >> 
> > >> 1. Propagate errors from proc_dointvec_minmax() instead of always
> > >>    returning success.  For example, writing non-integer garbage to the
> > >>    sysctl now returns an error instead of silently succeeding with
> > >>    unchanged values.
> > > 
> > > AI review suggest that this caused a new problem:
> > > 
> > > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
> > > 
> > > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> > > temporary then copy that into sysctl_lowmem_reserve_ratio if all
> > > proc_dointvec_minmax() returns "OK".
> > 
> > Seeing the v4 [1] it seems easier to keep the current fixup code until
> > proc_dointvec_minmax() is fixed.
> 
> 1. V3 -vs- V4:
>   I would prefer V4 as it actually prevents the partial write of the
>   vector in case of an error & returns that error back to user space.
>   Whereas V3 returns the error back to user space but keeps the partial
>   write.
> 
> That patterns of using a temp ctltable entry is seldom used but not
> unheard of.
> 
> > 
> > > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
> > > shouldn't update the table data until all the data has been validated.

Additionally, this will address the case where there is a partial write
to a vector where the input is erroneous. There is still a possibility
of having a "valid" partial write if you pass a set of valid values that
is less than the size of the vector.

> > 
> > I agree. What do the maintainers think?
> 
> I agree. The arrays being changed are not too big so we can easily have
> a staging variable that that gets written when all validations are done.
> The only "con" that I see for this solution is for when these
> proc_handlers get used with temp variables; in these cases we will be
> staging an already staging variable.
> 
> I have added this to my Todos
> 
> Best
> 
> -- 
> 
> Joel

On 8/4/26 14:20, Joel Granados wrote:
> On Mon, Aug 03, 2026 at 10:27:46AM +0200, Vlastimil Babka (SUSE) wrote:
>> +Cc: sysctl maintainers
>> 
>> On 8/1/26 20:44, Andrew Morton wrote:
>> > On Sat,  1 Aug 2026 23:11:25 +0800 Jianlin Shi <shijianlin11@foxmail.com> wrote:
>> > 
>> >> lowmem_reserve_ratio_sysctl_handler() ignores the return value of
>> >> proc_dointvec_minmax() and always calls setup_per_zone_lowmem_reserve(),
>> >> even for read operations.
>> >> 
>> >> Fix two issues:
>> >> 
>> >> 1. Propagate errors from proc_dointvec_minmax() instead of always
>> >>    returning success.  For example, writing non-integer garbage to the
>> >>    sysctl now returns an error instead of silently succeeding with
>> >>    unchanged values.
>> > 
>> > AI review suggest that this caused a new problem:
>> > 
>> > 	https://sashiko.dev/#/patchset/tencent_1BB7A5C4D5EEA67346634417753190E92A09@qq.com
>> > 
>> > Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
>> > temporary then copy that into sysctl_lowmem_reserve_ratio if all
>> > proc_dointvec_minmax() returns "OK".
>> 
>> Seeing the v4 [1] it seems easier to keep the current fixup code until
>> proc_dointvec_minmax() is fixed.
> 
> 1. V3 -vs- V4:
>   I would prefer V4 as it actually prevents the partial write of the
>   vector in case of an error & returns that error back to user space.
>   Whereas V3 returns the error back to user space but keeps the partial
>   write.
> 
> That patterns of using a temp ctltable entry is seldom used but not
> unheard of.
> 
>> 
>> > But really this is a flaw in proc_dointvec_minmax() isn't it?  It
>> > shouldn't update the table data until all the data has been validated.
>> 
>> I agree. What do the maintainers think?
> 
> I agree. The arrays being changed are not too big so we can easily have
> a staging variable that that gets written when all validations are done.
> The only "con" that I see for this solution is for when these
> proc_handlers get used with temp variables; in these cases we will be
> staging an already staging variable.

Maybe have a variant of the function e.g. proc_dointvec_minmax() that takes
an extra parameter pointing to the staging array? That way the caller can
allocate it on stack according to its needs (like v4 does) but there's no
fiddling with a temp ctltable entry. That way the core code doesn't need a
staging variable big enough for everyone, and users can be converted to the
new variant, and there's not double staging at any point.

> I have added this to my Todos
> 
> Best
>

On Sat, 1 Aug 2026 11:44:34 -0700 Andrew Morton wrote:
> Not sure what to do here.  Perhaps pass proc_dointvec_minmax() a
> temporary then copy that into sysctl_lowmem_reserve_ratio if all
> proc_dointvec_minmax() returns "OK".

Hi Andrew,

Thanks for the suggestion.  v4 uses a temporary ratio[] on write and
only copies into sysctl_lowmem_reserve_ratio[] and calls
setup_per_zone_lowmem_reserve() after the full parse succeeds.

I'll send [PATCH v4] in a separate mail shortly.

Thanks,
Jianlin