Re: [PATCH v2] perf_event_open03: Track SUnreclaim growth instead of MemAvailable

On 22/7/26 1:41 pm, Tang Yizhou via ltp wrote:
> From: Tang Yizhou <[email protected]>
> 
> CVE-2020-25704 leaks a small kmalloc() allocation (the filter's file
> name string) on every failed PERF_EVENT_IOC_SET_FILTER call. The test
> detected this by watching MemAvailable in /proc/meminfo drop by more
> than 100MB over 12M iterations.
> 
> MemAvailable is a global, system-wide estimate. It is heavily influenced
> by other memory activities of unrelated processes. This produces
> intermittent false positives: the test passes when run alone but
> occasionally fails when run with other tasks.
> 
> An improvement is to track the growth of SUnreclaim instead. It is less
> sensitive to other kinds of memory activities.
> 
> Also note in the failure output that unreclaimable slab can still grow
> due to unrelated slab activities, so a failure should be confirmed on an
> idle system or with kmemleak before being treated as a regression.
> 
> Reviewed-by: Vlastimil Babka (SUSE) <[email protected]>
> Reviewed-by: Cyril Hrubis <[email protected]>
> Signed-off-by: Tang Yizhou <[email protected]>
> ---
> v2:
> Take Vlastimil and Cyril's reviewed-by tag.
> Update the printed error message.
> 
>  .../perf_event_open/perf_event_open03.c       | 30 +++++++++++--------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c b/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c
> index 389cc35111b4..a4ce6eaffc02 100644
> --- a/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c
> +++ b/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c
> @@ -77,13 +77,13 @@ static void check_progress(int i)
>  
>  static void run(void)
>  {
> -	long diff, diff_total, mem_avail, mem_avail_prev;
> +	long diff, diff_total, slab, slab_prev;
>  	int i, sample;
>  
>  	sample = 0;
>  	diff_total = 0;
>  
> -	mem_avail_prev = SAFE_READ_MEMINFO("MemAvailable:");
> +	slab_prev = SAFE_READ_MEMINFO("SUnreclaim:");
>  	tst_timer_start(CLOCK_MONOTONIC);
>  
>  	/* leak about 100MB of RAM */
> @@ -92,28 +92,34 @@ static void run(void)
>  		check_progress(i);
>  
>  		/*
> -		 * Every 1200000 iterations, calculate the difference in memory
> -		 * availability. If the difference is greater than 20 * 1024 (20MB),
> -		 * increment the sample counter and log the event.
> +		 * Every 1200000 iterations, calculate how much the unreclaimable
> +		 * slab has grown. If the increase is greater than 20 * 1024
> +		 * (20MB), increment the sample counter and log the event.
>  		 */
>  		if ((i % 1200000) == 0) {
> -			mem_avail = SAFE_READ_MEMINFO("MemAvailable:");
> -			diff = mem_avail_prev - mem_avail;
> +			slab = SAFE_READ_MEMINFO("SUnreclaim:");
> +			diff = slab - slab_prev;
>  			diff_total += diff;
>  
>  			if (diff > 20 * 1024) {
>  				sample++;
> -				tst_res(TINFO, "MemAvailable decreased by %ld kB at iteration %d", diff, i);
> +				tst_res(TINFO, "SUnreclaim increased by %ld kB at iteration %d", diff, i);
>  			}
>  
> -			mem_avail_prev = mem_avail;
> +			slab_prev = slab;
>  		}
>  	}
>  
> -	if ((sample > 5) || (diff_total > 100 * 1024))
> -		tst_res(TFAIL, "Likely kernel memory leak detected, total decrease: %ld kB", diff_total);
> -	else
> +	if ((sample > 5) || (diff_total > 100 * 1024)) {
> +		tst_res(TFAIL,
> +			"Likely kernel memory leak detected, SUnreclaim increased by %ld kB total",
> +			diff_total);
> +		tst_res(TINFO,
> +			"Unreclaimable slab can also grow due to unrelated reasons as well. "
> +			"You can rerun the test with CONFIG_DEBUG_KMEMLEAK to make sure the leak is real");
> +	} else {
>  		tst_res(TPASS, "No memory leak found");
> +	}
>  }
>  
>  static void cleanup(void)

Hi, just checking. Would this patch be ready to merge?

-- 
Best Regards,
Yi


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp