Re: [RFC PATCH v1 08/25] unwind_user/eh_frame: Store .eh_frame_hdr section data in per-mm maple tree

On 8/18/2026 5:08 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Critical] The `eh_frame_mt` maple tree is initialized without `MT_FLAGS_USE_RCU`, causing internal nodes to be synchronously freed, which crashes concurrent lockless readers.

Josh, Steven, this seems like a valid finding.  See below.

> - [High] fork() fails to inherit .eh_frame_hdr registrations, breaking unwinding for child processes.

This is resolved by subsequent patch "unwind_user/eh_frame: Duplicate
registered .eh_frame[_hdr] section data on clone/fork".

> - [High] A TOCTOU race condition in `eh_frame_remove_section` allows a double-free of one section and a memory leak of another.

Steven, we somehow started to use the mmap_read_lock/mmap_write_lock to
synchronize adding/removing sections into the maple tree in the sframe
series, which my eh_frame series is missing:
https://lore.kernel.org/all/[email protected]/

I think using the advanced maple tree API would be preferable.  See below.

> - [High] An integer overflow in `table_end` calculation allows maliciously crafted `.eh_frame_hdr` sections to bypass bounds checking.

This is fixed in preceding patch "[RFC PATCH v1 07/25] unwind_user/
eh_frame: Add support for reading .eh_frame_hdr section".

> - [Low] Appending INIT_MM_EH_FRAME in init_mm causes a missing comma syntax error on multiple architectures.

IIRC this can be left for affected architectures to fix?

>> diff --git a/include/linux/eh_frame.h b/include/linux/eh_frame.h

>> +#define INIT_MM_EH_FRAME .eh_frame_mt = MTREE_INIT(eh_frame_mt, 0),

#define EH_FRAME_MT_FLAGS (MT_FLAGS_USE_RCU)

#define INIT_MM_EH_FRAME .eh_frame_mt = MTREE_INIT(eh_frame_mt, EH_FRAME_MT_FLAGS),

> 
> [Severity: Critical]
> Is the MT_FLAGS_USE_RCU flag required here?
> 
> Since readers are locklessly iterating the tree using mt_for_each() (which
> relies on rcu_read_lock()), omitting MT_FLAGS_USE_RCU causes concurrent
> mtree_erase() or mtree_insert_range() to synchronously free internal nodes via
> kfree() instead of kfree_rcu(). Can this cause a use-after-free for the
> concurrent readers?

>> diff --git a/kernel/fork.c b/kernel/fork.c

>> +static void mm_init_eh_frame(struct mm_struct *mm)
>> +{
>> +#ifdef CONFIG_HAVE_UNWIND_USER_EH_FRAME
>> +	mt_init(&mm->eh_frame_mt);

	mt_init_flags(&mm->eh_frame_mt, EH_FRAME_MT_FLAGS);

> 
> [Severity: Critical]
> Should this use mt_init_flags() with MT_FLAGS_USE_RCU?
> 
> Similar to the INIT_MM_EH_FRAME macro, initializing without RCU could lead to
> a use-after-free if internal maple tree nodes are freed while lockless readers
> are iterating.
...
>> +#endif
>> +}

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

>> +static int __eh_frame_remove_section(struct mm_struct *mm,

static int __eh_frame_remove_section(struct ma_state *mas,

>> +				     struct eh_frame_section *sec)
>> +{
>> +	if (!mtree_erase(&mm->eh_frame_mt, sec->text_start)) {
>> +		dbg("mtree_erase failed: text=%lx\n", sec->text_start);

	if (mas_erase(&mas) != sec) {
		dbg("mas_erase failed: text=%lx\n", sec->text_start);

>> +		return -EINVAL;
>> +	}
>> +
>> +	call_srcu(&eh_frame_srcu, &sec->rcu, eh_frame_free_srcu);
>> +
>> +	return 0;
>> +}
>> +
>>  int eh_frame_remove_section(unsigned long eh_frame_hdr_start)
>>  {
>> -	return -ENOSYS;
>> +	struct mm_struct *mm = current->mm;
>> +	struct eh_frame_section *sec;

	MA_STATE(mas, &mm->eh_frame_mt, 0, 0);

>> +	bool found = false;
>> +	int ret = 0;
>> +
>> +	guard(srcu)(&eh_frame_srcu);
>> +
>> +	mt_for_each(&mm->eh_frame_mt, sec, index, ULONG_MAX) {

	mtree_lock(&mm->eh_frame_mt);
	mas_for_each(&mas, sec, ULONG_MAX) {

>> +		if (sec->eh_frame_hdr_start == eh_frame_hdr_start) {
>> +			found = true;
>> +			ret |= __eh_frame_remove_section(mm, sec);
>> +		}
>> +	}

	mtree_unlock(&mm->eh_frame_mt);
	
> 
> [Severity: High]
> Can this mtree_erase() and call_srcu() sequence race with another thread and
> cause a double-free?
> 
> If Thread A locklessly iterates and finds sec1 in mt_for_each(), and Thread B
> concurrently removes sec1, Thread C could map a new section sec2 at the same
> text_start address.
> 
> When Thread A calls __eh_frame_remove_section(mm, sec1), mtree_erase() would
> successfully remove and return sec2. However, Thread A then passes the
> original sec1 pointer to call_srcu(), double-freeing sec1 and leaking sec2.

Valid.  Using the internal maple tree API to guard the whole iteration
and deletion of the eh_frame sections using the internal writer spinlock
should resolve this issue.

>> diff --git a/mm/init-mm.c b/mm/init-mm.c

>> @@ -48,6 +49,7 @@ struct mm_struct init_mm = {
>>  #endif
>>  	.flexible_array	= MM_STRUCT_FLEXIBLE_ARRAY_INIT,
>>  	INIT_MM_CONTEXT(init_mm)
>> +	INIT_MM_EH_FRAME
>>  };
> 
> [Severity: Low]
> Does appending INIT_MM_EH_FRAME here cause a missing comma syntax error on
> some architectures?
> 
> While x86's INIT_MM_CONTEXT was updated to include a trailing comma in this
> patch, other architectures (like UML or PowerPC Book3s 32-bit) may lack this
> trailing comma in their INIT_MM_CONTEXT definitions, breaking the build.

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
[email protected] / [email protected]

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/