From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta1.migadu.com (out-82.mta1.migadu.com [95.215.58.82]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 65F2E2B9B7 for ; Tue, 18 Aug 2026 15:06:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.82 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787065589; cv=none; b=lk8qTBFQeckaLxeFDW9oAq54rNbuDc3tGWmBC4+/BD89i6QCFBX6dGaXxekqg/bQzztcNf6fmsKRvMgeYmKVNC6wB/uUEZOotIPNdXdlZ3K5WgVuuSErKIkDTypLssKVaaTGP1zeaqFwD97cOBPQVGEZyNG/aNjQSKmhwiVf1bI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787065589; c=relaxed/simple; bh=cPHeBY5KTM96nrxv4hgyFw1PuHSjJCWzO7Hqsh/Ysd0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=X+5rJXC0gA5uDAgtta4K0ITENT0uflp9JsBqVTnTxZplHhwIRwlR8Mr8WQmmEz1VBvBVDBItAFCHVTz6xAQiWeGWyprFHF4Ax2/zPawRfBXtu7D1o8tIcp/l0i6EJDRcdrDiXBBJ8wPYY3ASp8ICs7FjHHFV7K/pyP/LKU8fCyo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=UrBeNWtT; arc=none smtp.client-ip=95.215.58.82 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="UrBeNWtT" X-Envelope-To: linux-kernel@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=cPHeBY5KTM96nrxv4hgyFw1PuHSjJCWzO7Hqsh/Ysd0=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1787065583; v=1; x=1787670383; b=UrBeNWtTGQAWeZcwku8B7wdpH44kvinFFN7TonhD1gMOPpyI7C1vHx2Njyp5slTMFeVJMW1H 5BSA59pZ6YJKyj41PWFSMNFLKr/19pY5Rknvgj18JgrZx9YrxL/EUVKvhaOe+k8QxXNyD5U3F9O MkOV4uaV0JBPvLV+lV8e0bUQ= X-Envelope-To: linux-kernel@vger.kernel.org Received: from localhost (2a03:2880:10ff:17::) by smtp.migadu.com with ESMTPS id 5b5f1f43086e936f; Tue, 18 Aug 2026 15:06:23 +0000 X-Migadu-Flow: FLOW_OUT Date: Tue, 18 Aug 2026 08:06:22 -0700 From: Shakeel Butt To: Michal Hocko Cc: Tao Cui , akpm@linux-foundation.org, linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, hannes@cmpxchg.org, roman.gushchin@linux.dev, muchun.song@linux.dev, Tao Cui Subject: Re: [PATCH] mm: page_counter: reject empty string in page_counter_memparse() Message-ID: References: <20260817042652.74136-1-cui.tao@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Tue, Aug 18, 2026 at 09:45:56AM +0200, Michal Hocko wrote: > 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 > > > > > > 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 > > > --- > > > 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. Mainly I was wondering if this is intentional or unintentional. If this us unintentional, can we fix it without anyone noticing? However if we are ok with this then let's make is formal and make this a documented behavior. I don't have any strong opinion either way but I think you are saying it safer to just assume this is intentional. Fine with me. Tao, can you send a patch to Documentation/admin-guide/cgroup-v2.rst which explicitly mention this behavior?