Re: [RFC PATCH v1 15/25] unwind_user/eh_frame: Duplicate registered .eh_frame[_hdr] section data on clone/fork

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Mixing `guard(srcu)` with `goto`-based error cleanup in `eh_frame_dup_mm()` violates kernel cleanup guidelines.
- [Low] Compilation failure in `eh_frame_validate_section()` when `CONFIG_EH_FRAME_VALIDATION` is enabled.
--

--- Patch [15]: [RFC PATCH v1 15/25] unwind_user/eh_frame: Duplicate registered .eh_frame[_hdr] section data on clone/fork ---
commit 5bac3cc6bbe2eca53f4122c35113146b44210ee7
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user/eh_frame: Duplicate registered .eh_frame[_hdr] section data on clone/fork

This commit ensures that a process's registered .eh_frame_hdr sections are
duplicated in the per-mm maple tree during virtual memory duplication. This
enables stack tracing for child processes using eh_frame.

> diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c
> index c9161229196ce..7f572d1711d3f 100644
> --- a/kernel/unwind/eh_frame.c
> +++ b/kernel/unwind/eh_frame.c

[ ... ]

> +int eh_frame_dup_mm(struct mm_struct *mm, struct mm_struct *oldmm)
> +{
> +	struct eh_frame_section *sec, *oldsec;
> +	unsigned long index = 0;
> +	int ret;
> +
> +	guard(srcu)(&eh_frame_srcu);
> +
> +	mt_for_each(&oldmm->eh_frame_mt, oldsec, index, ULONG_MAX) {
> +		sec = kzalloc(sizeof(*sec), GFP_KERNEL_ACCOUNT);
> +		if (!sec)
> +			return -ENOMEM;
> +
> +		__eh_frame_dup_section(sec, oldsec);
> +
> +		ret = mtree_insert_range(&mm->eh_frame_mt,
> +					 sec->text_start,
> +					 sec->text_end - 1,
> +					 sec, GFP_KERNEL_ACCOUNT);
> +		if (ret)
> +			goto err_free;
> +	}
> +
> +	return 0;
> +
> +err_free:
> +	free_section(sec);
> +	return ret;
> +}

[Severity: Low]
This isn't a bug, but does this function mix scope-based cleanup with
traditional goto-based error handling?

The kernel's cleanup guidelines specify that usage of goto and cleanup
helpers like guard() should not be mixed in the same function. Could this
be converted to use scope-based cleanup entirely, or avoid guard() to keep
the style consistent?

[ ... ]

Additionally, looking at eh_frame_validate_section() in the same file:

> 	if (!sec->has_binary_search_table)
> 		return 0;

[Severity: Low]
Can this cause a compilation failure when CONFIG_EH_FRAME_VALIDATION is
enabled?

It appears the function attempts to access sec->has_binary_search_table,
but this boolean member is not declared in struct eh_frame_section in
include/linux/eh_frame.h.

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