Re: [PATCH] mm: page_counter: reject empty string in page_counter_memparse()

On Mon 17-08-26 09:16:40, Shakeel Butt wrote:
> On Mon, Aug 17, 2026 at 12:26:52PM +0800, Tao Cui wrote:
> > From: Tao Cui <[email protected]>
> > 
> > memparse() consumes no characters on an empty input and leaves the
> > end pointer at the terminating NUL.  The only validation in
> > page_counter_memparse() checks for trailing characters, so an empty
> > input slips through and the limit becomes 0.
> > 
> > All limit write callbacks of the memory controller strstrip() the
> > input before calling this helper, so a script that writes an unset
> > variable hits this path:
> > 
> >   LIMIT=
> >   echo "$LIMIT" > $CG/memory.max
> >   echo $?
> >   0
> >   cat $CG/memory.max
> >   0
> > 
> > Nothing reports the mistake: the limit is now 0 and the OOM killer
> > goes after every task in the cgroup.  The same happens for
> > memory.min, memory.low, memory.high, memory.swap.high,
> > memory.swap.max and memory.zswap.max, where 0 silently removes the
> > protection or disables swap and zswap.
> > 
> > Reject the input when no characters were consumed, which is the one
> > case the trailing-character check cannot catch.
> > 
> > Fixes: 3e32cb2e0a12 ("mm: memcontrol: lockless page counters")
> > Signed-off-by: Tao Cui <[email protected]>
> > ---
> >  mm/page_counter.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/page_counter.c b/mm/page_counter.c
> > index 661e0f2a5127..d14db705b04f 100644
> > --- a/mm/page_counter.c
> > +++ b/mm/page_counter.c
> > @@ -281,7 +281,7 @@ int page_counter_memparse(const char *buf, const char *max,
> >  	}
> >  
> >  	bytes = memparse(buf, &end);
> > -	if (*end != '\0')
> > +	if (*end != '\0' || end == buf)
> >  		return -EINVAL;
> 
> I wonder if someone started depending on this behavior. In that case it is
> better to return error instead of silently ignore, so we will hear complains
> loudly. This looks good to me.

This is backward incompatible change and I am wondering why should we
even risk regression.
 
> Acked-by: Shakeel Butt <[email protected]>

-- 
Michal Hocko
SUSE Labs