Re: [PATCH v14 10/19] unwind_user/sframe: Remove .sframe section on detected corruption

Re: [PATCH v14 10/19] unwind_user/sframe: Remove .sframe section on detected corruption

[Jens Remus]

On 5/5/2026 10:39 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Concurrent stack unwinding on a corrupted `.sframe` section triggers a user-controllable `WARN_ON_ONCE()`, leading to a potential Denial of Service (DoS).
> --
> 
> commit a00e313eeb121415a3ca3fe4ed3b15126f46c76a
> Author: Josh Poimboeuf <jpoimboe@kernel.org>
> 
> unwind_user/sframe: Remove .sframe section on detected corruption
> 
> This commit removes the .sframe section on demand when corruption is
> detected during unwinding, to avoid continued attempted use of a bad
> section.
> 
>> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
>> index 9c755965ab837..8eed6a7d96256 100644
>> --- a/kernel/unwind/sframe.c
>> +++ b/kernel/unwind/sframe.c
>> @@ -358,6 +358,10 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
>>  	ret = __find_fre(sec, &fde, ip, frame);
>>  end:
>>  	user_read_access_end();
>> +
>> +	if (ret == -EFAULT)
>> +		WARN_ON_ONCE(sframe_remove_section(sec->sframe_start));
>> +
>>  	return ret;
>>  }
> 
> Can this WARN_ON_ONCE() be triggered concurrently by userspace?
> 
> If two threads within the same process are being unwound simultaneously,
> they could both encounter the same bad .sframe section and hit the
> -EFAULT path.
> 
> Because sframe_find() is only protected by the sframe_srcu read lock,
> both threads might call sframe_remove_section() at the same time. The first
> thread will successfully remove the section and return 0.
> 
> The second thread will fail to find the section in mm->sframe_mt:
> 
> kernel/unwind/sframe.c:sframe_remove_section() {
>     ...
> 	mt_for_each(&mm->sframe_mt, sec, index, ULONG_MAX) {
> 		if (sec->sframe_start == sframe_start) {
> 			found = true;
> 			ret |= __sframe_remove_section(mm, sec);
> 		}
> 	}
> 
> 	if (!found || ret)
> 		return -EINVAL;
>     ...
> }
> 
> Since found is false, this returns -EINVAL. Because -EINVAL is non-zero,
> WARN_ON_ONCE(-EINVAL) evaluates to true.
> 
> Could this allow an unprivileged user to intentionally trigger kernel
> warnings (and panic the system if panic_on_warn is set) by unmapping a
> registered .sframe section and triggering concurrent unwinds?

Please advise.

Should all WARN_ON_ONCE() in user unwind (sframe) be replaced with e.g.
dbg_once() instead, defining it as follows in sframe_debug.h:

#define dbg_once(fmt, ...)							\
	pr_debug_once("%s (%d): " fmt, current->comm, current->pid, ##__VA_ARGS__)

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/