Re: [RFC PATCH v1 14/25] unwind_user/eh_frame: Add .eh_frame[_hdr] validation option

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

On 8/18/2026 5:08 PM, sashiko-bot@kernel.org wrote:

>> diff --git a/arch/Kconfig b/arch/Kconfig

>> +config EH_FRAME_VALIDATION
>> +	bool "Enable .eh_frame[_hdr] section debugging"
>> +	depends on HAVE_UNWIND_USER_EH_FRAME
>> +	depends on DYNAMIC_DEBUG
>> +	help
>> +	  When adding an .eh_frame_hdr section for a test, validate the
>> +	  entire section and its referenced entrire .eh_frame section
> 
> [Severity: Low]
> Is there a typo here? (entrire -> entire)

Fixed in next version.

> 
>> +	  immediately rather than on demand.
> 
> [ ... ]

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

>> +#ifdef CONFIG_EH_FRAME_VALIDATION
>> +
>> +static int eh_frame_validate_section(struct eh_frame_section *sec)
>> +{
>> +	void __user *table_start_ptr;
>> +	unsigned long table_size;
>> +	u8 table_enc;
>> +	int entry_size;
>> +	unsigned long prev_func_addr;
>> +	unsigned int i;

	unsigned long i;

>> +
>> +	if (!sec->has_binary_search_table)
> 
> [Severity: Low]
> Does this cause a build failure? It appears has_binary_search_table was never
> added to struct eh_frame_section.

That was a rebase error.  Fixed in next version.

> 
>> +		return 0;
>> +
>> +	if (!sec->fde_count) {
>> +		dbg_sec(".eh_frame_hdr: invalid FDE count\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	table_enc = sec->binary_search_table_enc;
>> +	entry_size = 2 * encoded_pointer_size(table_enc);
>> +	if (!entry_size) {
>> +		dbg_sec(".eh_frame_hdr: invalid binary search table entry size\n");
>> +		return -EINVAL;
>> +	}
>> +	table_start_ptr = (void __user *)sec->binary_search_table_start;
>> +	table_size = sec->binary_search_table_end - sec->binary_search_table_start;
>> +
>> +	for (i = 0; i < sec->fde_count; i++) {
> 
> [Severity: High]
> Can this loop become infinite and cause a hard lockup?
> 
> The loop counter i is a 32-bit unsigned int, while sec->fde_count is a 64-bit
> unsigned long. If sec->fde_count exceeds UINT_MAX, i will wrap around to 0, and
> the condition i < sec->fde_count will always evaluate to true.

My bad!  Fixed in next version.

> 
> [Severity: Medium]
> Additionally, can this unbounded loop over potentially millions of entries
> cause a CPU soft lockup or RCU stall? 
> 
> This loop executes entirely without a cond_resched() call, allowing a local
> user to load a moderately large ELF file and hold the CPU uninterrupted for
> seconds.

IIRC Steven told me not to add cond_resched() in reply to previous
Sashiko AI reviews of the unwind user sframe series.  Won't address
for now.

> 
>> +		struct eh_frame_fde fde;
>> +		unsigned long cur;
>> +		unsigned long func_addr, fde_addr;
>> +		int ret;
>> +
>> +		cur = sec->binary_search_table_start + i * entry_size;
>> +
>> +		scoped_user_read_access_size(table_start_ptr, table_size, Efault) {
>> +			/* Read function start address from table */
>> +			ret = read_encoded_pointer(sec, NULL, &cur,
>> +						   sec->binary_search_table_end,
>> +						   table_enc, &func_addr);
>> +			if (ret) {
>> +				dbg_sec_ehfh(cur, "table[%u]: failed to read function start address\n", i);
> 
> [Severity: High]
> Does calling dbg_sec_ehfh() inside scoped_user_read_access_size violate UACCESS
> rules?
> 
> The macro scoped_user_read_access_size enables UACCESS. dbg_sec_ehfh expands to
> pr_debug, which calls printk. Calling complex or sleepable functions like
> printk with UACCESS enabled can trigger page faults, take locks, or schedule,
> potentially leading to kernel oopses or panics.

This is mentioned in the patch notes.  I am looking for suggestions on
how to emit debug messages from a scoped UACCESS region.  Is the only
option to change to code from the unsafe to the safe versions of the
user access functions?

> 
>> +				return ret;
>> +			}
> 

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/