linux-kernel - Re: [PATCH v7 08/17] unwind_user/deferred: Add unwind cache

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: <aBc1cqWcTJmeqg3X@gmail.com>
Date: Sun, 4 May 2025 11:37:54 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>, x86@...nel.org,
	Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>
Subject: Re: [PATCH v7 08/17] unwind_user/deferred: Add unwind cache


* Steven Rostedt <rostedt@...dmis.org> wrote:

> -struct unwind_task_info {
> +struct unwind_cache {
>  	unsigned long		*entries;
> +	unsigned int		nr_entries;
> +};
> +
> +struct unwind_task_info {
> +	struct unwind_cache	cache;
>  };

> @@ -19,17 +20,29 @@ int unwind_deferred_trace(struct unwind_stacktrace *trace)
>  	if (current->flags & PF_EXITING)
>  		return -EINVAL;
>  
> -       if (!info->entries) {
> -               info->entries = kmalloc_array(UNWIND_MAX_ENTRIES, sizeof(long),
> -                                             GFP_KERNEL);
> -               if (!info->entries)
> -		       return -ENOMEM;
> +	if (!cache->entries) {
> +		cache->entries = kmalloc_array(UNWIND_MAX_ENTRIES, sizeof(long),
> +					       GFP_KERNEL);
> +		if (!cache->entries)
> +			return -ENOMEM;
> +        }

That's just sloppy: why isn't the unwind_cache a pointer to begin with, 
with the dynamically allocated object containing ::nr_entries?

Also, the code has whitespace damage.

> +
> +	trace->entries = cache->entries;
> +
> +	if (cache->nr_entries) {
> +               /*
> +                * The user stack has already been previously unwound in this
> +                * entry context.  Skip the unwind and use the cache.
> +                */
> +               trace->nr = cache->nr_entries;
> +               return 0;

Whitespace damage here too. Maybe in other patches as well.

Please don't rush this series without first reviewing it carefully ...

Thanks,

	Ingo