linux-kernel - Re: [PATCH] ftrace: Do not over-allocate ftrace memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260106161844.02ab387f@gandalf.local.home>
Date: Tue, 6 Jan 2026 16:18:44 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mark Rutland
 <mark.rutland@....com>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] ftrace: Do not over-allocate ftrace memory

On Tue,  6 Jan 2026 12:35:33 -0800
Guenter Roeck <linux@...ck-us.net> wrote:

> The pg_remaining calculation in ftrace_process_locs() assumes that
> ENTRIES_PER_PAGE multiplied by 2^order equals the actual capacity of the
> allocated page group. However, ENTRIES_PER_PAGE is PAGE_SIZE / ENTRY_SIZE
> (integer division). When PAGE_SIZE is not a multiple of ENTRY_SIZE (e.g.
> 4096 / 24 = 170 with remainder 16), high-order allocations (like 256 pages)
> have significantly more capacity than 256 * 170. This leads to pg_remaining
> being underestimated, which in turn makes skip (derived from skipped -
> pg_remaining) larger than expected, causing the WARN(skip != remaining)
> to trigger.

Nice catch! I guess you have a machine that allows much higher order
allocations than I do ;-)

> 
> Extra allocated pages for ftrace: 2 with 654 skipped
> WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7295 ftrace_process_locs+0x5bf/0x5e0
> 
> A similar problem in ftrace_allocate_records() can result in allocating
> too many pages. This can trigger the second warning in
> ftrace_process_locs().
> 
> Extra allocated pages for ftrace
> WARNING: CPU: 0 PID: 0 at kernel/trace/ftrace.c:7276 ftrace_process_locs+0x548/0x580
> 
> Use the actual capacity of a page group to determine if too many pages
> have been allocated to solve the problem. Also use the actual capacity
> of a page group to determine the number of pages needed to avoid over-
> allocations in ftrace_allocate_records().
> 
> Fixes: 4a3efc6baff93 ("ftrace: Update the mcount_loc check of skipped entries")
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
> ---
>  kernel/trace/ftrace.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index ef2d5dca6f70..211ec7a04f7e 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3844,7 +3844,7 @@ static int ftrace_allocate_records(struct ftrace_page *pg, int count)
>  		return -EINVAL;
>  
>  	/* We want to fill as much as possible, with no empty pages */
> -	pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);
> +	pages = DIV_ROUND_UP(count * ENTRY_SIZE, PAGE_SIZE);
>  	order = fls(pages) - 1;
>  
>   again:
> @@ -7308,24 +7308,33 @@ static int ftrace_process_locs(struct module *mod,
>  		unsigned long skip;
>  
>  		/* Count the number of entries unused and compare it to skipped. */
> -		pg_remaining = (ENTRIES_PER_PAGE << pg->order) - pg->index;
> +		pg_remaining = (PAGE_SIZE << pg->order) / ENTRY_SIZE - pg->index;
>  
>  		if (!WARN(skipped < pg_remaining, "Extra allocated pages for ftrace")) {
> +			unsigned long space = 0;
>  
>  			skip = skipped - pg_remaining;
>  
> -			for (pg = pg_unuse; pg; pg = pg->next)
> +			for (pg = pg_unuse; pg; pg = pg->next) {
>  				remaining += 1 << pg->order;
> +				/*
> +				 * The capacity of a page group is
> +				 *     (PAGE_SIZE << order) / ENTRY_SIZE
> +				 * Accumulate the total capacity of unused pages.
> +				 */
> +				space += (PAGE_SIZE << pg->order) / ENTRY_SIZE;
> +			}
>  
>  			pages -= remaining;

I think pages is meaningless here, as it was set in the beginning with:

	pages = DIV_ROUND_UP(count, ENTRIES_PER_PAGE);

Which is incorrect. I wonder if we should set it via:

	/*
	 * Use ftrace_number_of_pages to determine how many pages were
	 * allocated
	 */
	pages = ftrace_number_of_pages;

	start_pg = ftrace_allocate_pages(count);
	if (!start_pg)
		return -ENOMEM;

	/* ftrace_allocate_pages() increments ftrace_number_of_pages */
	pages = ftrace_number_of_pages - pages;

This will make pages equal the number of pages that were allocated. Then
I'm not sure we need this extra logic.

-- Steve


>  
> -			skip = DIV_ROUND_UP(skip, ENTRIES_PER_PAGE);
> -
>  			/*
> -			 * Check to see if the number of pages remaining would
> -			 * just fit the number of entries skipped.
> +			 * Check to see if extra pages have been allocated.
> +			 * Only warn if the number of unused entries is larger
> +			 * than the number of entries per page to avoid false
> +			 * positives due to rounding.
>  			 */
> -			WARN(skip != remaining, "Extra allocated pages for ftrace: %lu with %lu skipped",
> +			WARN(space - skip > ENTRIES_PER_PAGE,
> +			     "Extra allocated pages for ftrace: %lu with %lu skipped",
>  			     remaining, skipped);
>  		}
>  		/* Need to synchronize with ftrace_location_range() */