linux-kernel - Re: [PATCH] fgraph: Give ret

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] [day] [month] [year] [list]

Message-ID: <e7a45262-ec83-4638-bd44-d5605c5b17a7@arm.com>
Date: Mon, 21 Oct 2024 10:29:28 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
 Linux trace kernel <linux-trace-kernel@...r.kernel.org>,
 Masami Hiramatsu <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH] fgraph: Give ret_stack its own kmem cache

On 21/10/2024 09:37, Steven Rostedt wrote:
> On Mon, 21 Oct 2024 08:58:11 +0100
> Ryan Roberts <ryan.roberts@....com> wrote:
> 
>>> @@ -1290,6 +1305,16 @@ int register_ftrace_graph(struct fgraph_ops *gops)
>>>  
>>>  	mutex_lock(&ftrace_lock);
>>>  
>>> +	if (!fgraph_stack_cachep)
>>> +		fgraph_stack_cachep = kmem_cache_create("fgraph_stack",
>>> +							SHADOW_STACK_SIZE,
>>> +							SHADOW_STACK_SIZE, 0, NULL);  
>>
>> (I don't have any experience with this code, but...) is there any value/need to
>> destroy the cache in unregister_ftrace_graph()? I guess you would need to
>> refcount it, so its created on the first call to register and destroyed on the
>> last call to unregister?
> 
> No, we can't destroy it. In fact, we can't destroy the stacks
> themselves until the task exits. This is because a function could have
> been traced and its return address gets replaced by the fgraph return
> code. Then it goes to sleep. For example, say you were tracing poll,
> and systemd did a poll and you traced it. Now it may be sleeping
> forever, waiting for some input. When it finally wakes up and exits the
> function, it will need to get its original return address back.
> 
> The ret_stack holds the original return address that is needed when the
> function finishes. Thus, its not safe to free it even when tracing is
> finished. The callbacks may not be called when tracing is done, but the
> ret_stack used to do the tracing will be called long after tracing is
> over.
> 
> Now I'm looking at being able to free stacks by scanning all the tasks
> after tracing is over and if the stack isn't being used (it's easy to
> know if it is or not) then we can free it. But for those cases where
> they are still being used, then we just have to give up and leave it be.

Ah, gotya. Thanks for the explanation!

> 
> -- Steve