On 8/18/2026 5:08 PM, [email protected] 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)
[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/
