Re: [PATCH v16 10/20] unwind_user/sframe: Remove .sframe section on detected corruption

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Concurrent removal of a corrupted or unmapped .sframe section triggers a user-controllable WARN_ON_ONCE.
- [High] Valid but paged-out .sframe sections are permanently removed due to transient page faults.
--

commit b574618cbc641ef52e2678be129e34a64bc4fb53
Author: Josh Poimboeuf <jpoimboe@kernel.org>

unwind_user/sframe: Remove .sframe section on detected corruption

To avoid continued attempted use of a bad .sframe section, this commit removes
it on demand when the first sign of corruption is detected.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 41ece3ca62a1f..e0eb2adf5a074 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c

[ ... ]

> @@ -371,10 +371,15 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
>  			ret = __find_fre(sec, &fde, ip, frame);
>  	}
>  
> +end:
> +	if (ret && ret != -ENOENT)
> +		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));

[Severity: High]
Can a concurrent unwind cause a user-controllable warning here?

If two threads concurrently fail to read a corrupted .sframe section, they 
could both reach this code. The first thread would successfully remove the 
section, but the second thread would fail to find it in the maple tree, 
causing sframe_remove_section() to return -EINVAL. 

This would trigger the WARN_ON_ONCE(), potentially allowing an unprivileged 
user to crash the system if panic_on_warn is enabled.

>  
>  	return ret;
>  
>  Efault:
> -	return -EFAULT;
> +	ret = -EFAULT;
> +	goto end;
>  }

[Severity: High]
Will this permanently remove perfectly valid .sframe sections if they happen 
to be paged out?

User space unwinding often happens in atomic contexts like perf NMIs where 
demand paging is not permitted. If a valid .sframe section is temporarily 
swapped out or hasn't been faulted in yet, the memory read operations will 
cleanly fail and jump here, setting ret to -EFAULT.

Since -EFAULT is not -ENOENT, the error handling logic jumps to the end label 
and treats this transient failure as permanent corruption, permanently removing 
the section from the maple tree. Applications might lose unwinding capabilities 
simply because their memory was temporarily paged out during a profiler sample.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521142546.3908498-1-jremus@linux.ibm.com?part=10