Re: [RFC PATCH v1 25/25] unwind_user/eh_frame: Add prctl() interface for (un)registering .eh_frame_hdr sections

[Jens Remus <jremus@linux.ibm.com>]

On 8/18/2026 5:17 PM, sashiko-bot@kernel.org wrote:
> This patch adds two new prctl options to allow dynamic linkers to register
> and unregister .eh_frame_hdr sections with the kernel. It introduces the
> eh_frame_setup structure to pass section bounds from userspace and adds the
> corresponding handlers.

>> diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c

>> +int eh_frame_register(struct eh_frame_setup __user *user_data, __kernel_size_t size)
>> +{
>> +	struct eh_frame_setup data;
>> +	unsigned long eh_frame_hdr_end, text_end;
>> +
>> +	if (!user_data && !size)
>> +		return -EINVAL;
>> +
>> +	if (size != sizeof(data))
>> +		return -EINVAL;
> 
> [Severity: Medium]
> Could this strict size check prevent future UAPI extensions from being handled
> properly by older kernels?
> 
> If a newer userspace binary passes a legitimately extended eh_frame_setup
> struct to an older kernel, it looks like it will be rejected with -EINVAL.
> 
> Would it be better to use copy_struct_from_user() here to safely handle
> zero-padded extensions?

Steven, IIRC you got similar review feedback on your stacktrace syscall
and rejected it.  Why wouldn't we copy sizeof(data) and check that any
additional size - sizeof(data) bytes of user data are zero?

> 
>> +
>> +	if (copy_from_user(&data, user_data, sizeof(data)))
>> +		return -EFAULT;
> 
> [ ... ]
> 
>> +int eh_frame_unregister(struct eh_frame_setup __user *user_data, __kernel_size_t size)
>> +{
>> +	struct eh_frame_setup data;
>> +
>> +	if (!user_data && !size)
>> +		return -EINVAL;
>> +
>> +	if (size != sizeof(data))
>> +		return -EINVAL;
> 
> [Severity: Medium]
> As with eh_frame_register(), does this strict size comparison break forward
> compatibility for future struct extensions?

Likewise.

> 
>> +
>> +	if (copy_from_user(&data, user_data, sizeof(data)))
>> +		return -EFAULT;
>> +
>> +	/* Unregister only uses eh_frame_hdr_start */
>> +	if (data.eh_frame_hdr_size || data.text_start || data.text_size)
>> +		return -EINVAL;
>> +
>> +	return eh_frame_remove_section(data.eh_frame_hdr_start);
> 
> [Severity: Medium]
> Does this code silently truncate the 64-bit parameter on 32-bit platforms,
> potentially causing the wrong section to be unregistered?
> 
> The data.eh_frame_hdr_start parameter is a __u64, but it is passed directly
> into eh_frame_remove_section() which takes an unsigned long. If userspace
> provides an out-of-bounds 64-bit value on a 32-bit system, it appears it
> would be implicitly truncated to a valid 32-bit address rather than being
> rejected.

Steven, do you have any suggestion?

> 
> Looking at the underlying implementation of eh_frame_remove_section() that is
> exposed by this new prctl, is there a race condition in
> __eh_frame_remove_section() that could lead to a double-free?

This should be resolved by the fixes to "[RFC PATCH v1 08/25] unwind_user/
eh_frame: Store .eh_frame_hdr section data in per-mm maple tree".

> 
> kernel/unwind/eh_frame.c:__eh_frame_remove_section() {
>     ...
>     if (!mtree_erase(&mm->eh_frame_mt, sec->text_start)) {
>         dbg_sec("mtree_erase failed: text=%lx\n", sec->text_start);
>         return -EINVAL;
>     }
>     
>     call_srcu(&eh_frame_srcu, &sec->rcu, eh_frame_free_srcu);
>     ...
> }
> 
> [Severity: Critical]
> If a thread unregisters the section and a third thread immediately registers
> a new section at the exact same address, would mtree_erase() erase and return
> the new section instead?
> 
> Because the code evaluates the return value simply as non-NULL, it seems it
> would proceed to call call_srcu() on the old section again, potentially
> causing a double-free and SRCU list corruption.

Likewise fixed in other patch.

> 
> Also, regarding the interaction with the unwinder fast path in eh_frame_find():
> 
> kernel/unwind/eh_frame.c:eh_frame_find() {
>     ...
>     if (ret && (ret != -ENOENT && ret != -EOPNOTSUPP)) {
>         dbg_sec("removing bad .eh_frame[_hdr] section\n");
>         if (eh_frame_remove_section(sec->eh_frame_hdr_start))
>     ...
> }
> 
> [Severity: Critical]
> Can this sequence cause a hard deadlock or NMI-safety violation?
> 
> Stack unwinding is frequently invoked from NMI or IRQ contexts. If
> eh_frame_find() encounters a parsing error, it unconditionally calls
> eh_frame_remove_section(). This attempts to acquire the mtree_lock spinlock
> and invokes call_srcu(), both of which are forbidden in NMI/IRQ contexts.

Deferred unwinding of user space does not run in NMI/IRQ context.

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/