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

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Race condition in `__eh_frame_remove_section()` leads to double-free, memory leak, and SRCU list corruption.
- [Critical] Unwinder error path triggers hard deadlock and NMI-safety violations by calling `eh_frame_remove_section()`.
- [Medium] Strict size matching in prctl handlers rejects UAPI structure extensions, breaking forward compatibility.
- [Medium] Silent data truncation of 64-bit `eh_frame_hdr_start` in `eh_frame_unregister()` on 32-bit platforms.
--

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

commit 04469d758625968e94108e567fd1bac500c0f41d
Author: Jens Remus [off-list ref]

unwind_user/eh_frame: Add prctl() interface for (un)registering .eh_frame_hdr sections

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.

Link: https://lore.kernel.org/all/20260528151023.00f5ec4e@gandalf.local.home/ (local)

[ ... ]

[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?

+
+	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?

+
+	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.

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?

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.

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.

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