Re: [Internet][PATCH] memcg: fix integer overflow in calculate_high_delay() bypassing memory.high throttle

[Bingfang Guo <bfguo@icloud.com>]

On Thu, Oct 29, 2026 at 03:38:56PM +0800, zhangjiao2 wrote:
> From: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> 
> calculate_high_delay() computes the throttling penalty as:
> 
>   penalty_jiffies = max_overage * max_overage * HZ;
> 
> where max_overage is a u64 derived from the usage/high ratio shifted
> left by MEMCG_DELAY_PRECISION_SHIFT (20).  When a cgroup exceeds its
> memory.high by a large factor (>= ~4096x), max_overage reaches 2^32
> and the squared multiplication overflows u64, wrapping to a small or
> zero value.  A zero penalty passes the 'penalty_jiffies <= HZ / 100'
> early-exit in __mem_cgroup_handle_over_high(), so the cgroup is never
> throttled despite being massively over limit.
> 
> This is exploitable for denial of service: set memory.high to a small
> value (e.g. 1 MB) and allocate several gigabytes; the overflow causes
> the throttling sleep to be skipped entirely.

Hi!

> 
> Additionally, on 32-bit kernels the 'unsigned long penalty_jiffies'
> intermediate truncates the u64 product before the right-shifts, which
> can also zero the result.
> 
> Fix both issues by:
> 
>   1. Capping max_overage at (1 << 20) before squaring.  Any overage
>      this large already produces a penalty far above
>      MEMCG_MAX_HIGH_DELAY_JIFFIES, which the caller clamps anyway, so
>      the cap is lossless.
> 
>   2. Using a u64 intermediate for penalty_jiffies so the right-shifts
>      operate on the full 64-bit value on all architectures.
> 
> The post-fix worst-case intermediate is (1<<20)^2 * HZ = 2^40 * HZ,
> which is ~2^50 for HZ=1000 and well within u64 range.
> 
> Signed-off-by: zhang jiao <zhangjiao2@cmss.chinamobile.com>
> ---
>  mm/memcontrol.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 84f4a8fd4d16..c0ab3940025c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2576,7 +2576,7 @@ static u64 swap_find_max_overage(struct mem_cgroup *memcg)
>  static unsigned long calculate_high_delay(unsigned int nr_pages,
>  					  u64 max_overage)
>  {
> -	unsigned long penalty_jiffies;
> +	u64 penalty_jiffies;

The return value is:

    return penalty_jiffies * nr_pages / MEMCG_CHARGE_BATCH;

And the type is still unsigned long, which can still truncate the return value
when nr_pages is quite large on 32-bit systems theoretically. So maybe you can
clamp it again:

    return min(..., MEMCG_MAX_HIGH_DELAY_JIFFIES);

Other than that, the patch looks good to me.

>  
>  	if (!max_overage)
>  		return 0;
> @@ -2588,7 +2588,16 @@ static unsigned long calculate_high_delay(unsigned int nr_pages,
>  	 * memcg in question makes it clear that it has no intention of stopping
>  	 * its crazy behaviour, so we exponentially increase the delay based on
>  	 * overage amount.
> +	 *
> +	 * Cap max_overage before squaring to prevent integer overflow that
> +	 * could otherwise wrap the penalty to zero and bypass throttling.
> +	 * Use a u64 intermediate so the right-shifts are not truncated on
> +	 * 32-bit kernels.  Any overage large enough to hit the cap already
> +	 * produces a penalty well above MEMCG_MAX_HIGH_DELAY_JIFFIES, which
> +	 * the caller enforces regardless, so capping is lossless.
>  	 */
> +	max_overage = min_t(u64, max_overage, 1ULL << 20);
> +
>  	penalty_jiffies = max_overage * max_overage * HZ;
>  	penalty_jiffies >>= MEMCG_DELAY_PRECISION_SHIFT;
>  	penalty_jiffies >>= MEMCG_DELAY_SCALING_SHIFT;
> -- 
> 2.50.1.windows.1
> 
> 
> 
> 

--
Regards,
Bingfang