linux-kernel - Re: [RFC PATCH v2 5/8] arm64: Detect an FTRACE frame and mark a stack trace unreliable

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 for Android: free password hash cracker in your pocket

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

Message-ID: <8aa50127-3f00-818d-d58c-4b3ff7235c74@linux.microsoft.com>
Date:   Tue, 23 Mar 2021 15:24:14 -0500
From:   "Madhavan T. Venkataraman" <madvenka@...ux.microsoft.com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     broonie@...nel.org, jpoimboe@...hat.com, jthierry@...hat.com,
        catalin.marinas@....com, will@...nel.org,
        linux-arm-kernel@...ts.infradead.org,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v2 5/8] arm64: Detect an FTRACE frame and mark a stack
 trace unreliable



On 3/23/21 1:30 PM, Mark Rutland wrote:
> On Tue, Mar 23, 2021 at 12:23:34PM -0500, Madhavan T. Venkataraman wrote:
>> On 3/23/21 12:02 PM, Mark Rutland wrote:
> 
> [...]
> 
>> I think that I did a bad job of explaining what I wanted to do. It is not
>> for any additional protection at all.
>>
>> So, let us say we create a field in the task structure:
>>
>> 	u64		unreliable_stack;
>>
>> Whenever an EL1 exception is entered or FTRACE is entered and pt_regs get
>> set up and pt_regs->stackframe gets chained, increment unreliable_stack.
>> On exiting the above, decrement unreliable_stack.
>>
>> In arch_stack_walk_reliable(), simply do this check upfront:
>>
>> 	if (task->unreliable_stack)
>> 		return -EINVAL;
>>
>> This way, the function does not even bother unwinding the stack to find
>> exception frames or checking for different return addresses or anything.
>> We also don't have to worry about code being reorganized, functions
>> being renamed, etc. It also may help in debugging to know if a task is
>> experiencing an exception and the level of nesting, etc.
> 
> As in my other reply, since this is an optimization that is not
> necessary for functional correctness, I would prefer to avoid this for
> now. We can reconsider that in future if we encounter performance
> problems.
> 
> Even with this there will be cases where we have to identify
> non-unwindable functions explicitly (e.g. the patchable-function-entry
> trampolines, where the real return address is in x9), and I'd prefer
> that we use one mechanism consistently.
> 
> I suspect that in the future we'll need to unwind across exception
> boundaries using metadata, and we can treat the non-unwindable metadata
> in the same way.
> 
> [...]
> 
>>> 3. Figure out exception boundary handling. I'm currently working to
>>>    simplify the entry assembly down to a uniform set of stubs, and I'd
>>>    prefer to get that sorted before we teach the unwinder about
>>>    exception boundaries, as it'll be significantly simpler to reason
>>>    about and won't end up clashing with the rework.
>>
>> So, here is where I still have a question. Is it necessary for the unwinder
>> to know the exception boundaries? Is it not enough if it knows if there are
>> exceptions present? For instance, using something like num_special_frames
>> I suggested above?
> 
> I agree that it would be legitimate to bail out early if we knew there
> was going to be an exception somewhere in the trace. Regardless, I think
> it's simpler overall to identify non-unwindability during the trace, and
> doing that during the trace aligns more closely with the structure that
> we'll need to permit unwinding across these boundaries in future, so I'd
> prefer we do that rather than trying to optimize for early returns
> today.
> 

OK. Fair enough.

Thanks.

Madhavan