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

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

quoted

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

quoted

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

quoted

+	  immediately rather than on demand.

[ ... ]

quoted

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

quoted

+#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;

quoted

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

quoted

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

quoted

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

quoted

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