Thread (67 messages) 67 messages, 3 authors, 2026-08-21

Re: [RFC PATCH v1 07/25] unwind_user/eh_frame: Add support for reading .eh_frame_hdr section

From: Jens Remus <hidden>
Date: 2026-08-21 19:41:13
Also in: linux-s390, sashiko-reviews

On 8/18/2026 5:02 PM, sashiko-bot@kernel.org wrote:
quoted
diff --git a/kernel/unwind/eh_frame.c b/kernel/unwind/eh_frame.c
quoted
+static __always_inline int read_encoded_pointer(struct eh_frame_section *sec,
+						unsigned long *addr,
+						unsigned long end,
+						u8 encoding,
+						unsigned long *value)
+{
+	unsigned long cur = *addr;
+	u8 format = DW_EH_PE_format(encoding);
+	u8 application = DW_EH_PE_application(encoding);
+	unsigned long result = 0;
+	int ret;
+
+	if (encoding == DW_EH_PE_omit)
+		return -EINVAL;
+
+	/* Determine base address based on application */
+	switch (application) {
+	case 0:
+		/* Absolute */
+		break;
+	case DW_EH_PE_pcrel:
+		result = *addr;
+		break;
+	case DW_EH_PE_datarel:
+		result = sec->eh_frame_hdr_start;
+		break;
+	case DW_EH_PE_textrel:
+		result = sec->text_start;
+		break;
+	case DW_EH_PE_funcrel:
+	case DW_EH_PE_aligned:
+		return -EOPNOTSUPP;
+	default:
+		return -EINVAL;
+	}
+
+	/* Read value based on format */
+	switch (format) {
+	case DW_EH_PE_absptr:
+		UNSAFE_GET_USER_INC(result, cur, end, Efault);
		unsigned long tmp;
		UNSAFE_GET_USER_INC(tmp, cur, end, Efault);
		result += tmp;
quoted
+		break;
[Severity: Medium]
Does this overwrite the base address that was just computed for relative
encodings?

When handling DW_EH_PE_absptr in combination with relative application flags
like DW_EH_PE_pcrel, the base address is stored in the result variable above.
However, UNSAFE_GET_USER_INC directly assigns the read value to result
instead of adding it, which will discard the base address.

Should this read into a temporary variable and add it to result, similar to
how the other formats are handled?
Yes.  I wrongly assumed that DW_EH_PE_absptr would be treated as-is,
similar as DW_EH_PE_omit, and would therefore not be allowed in
combination with any application flags as well as DW_EH_PE_indirect.

The LSB spec, section DWARF EH encoding [1] does read as any combination
would be allowed, as it is also implemented in libgcc's unwinder.

[1]: LSB 5.0.0, 10.5.1. DWARF Exception Header Encoding
     https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/dwarfext.html#DWARFEHENCODING> 
quoted
+	case DW_EH_PE_uleb128: {
+		unsigned long tmp;
+		ret = read_uleb128(&cur, end, &tmp);
+		if (ret)
+			return ret;
+		result += tmp;
+		break;
+	}
quoted
+static int eh_frame_read_header(struct eh_frame_section *sec)
+{
+	struct mm_struct *mm = current->mm;
+	void __user *eh_frame_hdr = (void __user *)sec->eh_frame_hdr_start;
+	unsigned long cur = sec->eh_frame_hdr_start, end = sec->eh_frame_hdr_end;
+	unsigned long eh_frame_start, eh_frame_vma_end, table_start, table_end;
+	u8 version, eh_frame_ptr_enc, fde_count_enc, table_enc;
+	unsigned long fde_count;
+	int entry_size;
+	int ret;
[ ... ]
quoted
+		/* Read FDE count */
+		ret = read_encoded_pointer(sec, &cur, end,
+					   fde_count_enc, &fde_count);
+		if (ret)
+			return ret;
+
+		/* Determine binary search table start and end */
+		table_start = cur;
+		entry_size = 2 * encoded_pointer_size(table_enc);
+		if (!entry_size)
+			return -EINVAL;
Check that the FDE count is within the remaining .eh_frame_hdr bounds:

		if (fde_count > (end - table_start) / entry_size)
			return -EINVAL;
quoted
+		table_end = table_start + fde_count * entry_size;
+		if (table_end > sec->eh_frame_hdr_end)
+			return -EINVAL;
Drop this check in favor of the new one above.
[Severity: Medium]
Can a maliciously large fde_count cause an integer overflow here?

Since fde_count is parsed from the untrusted user eh_frame_hdr section,
the multiplication of fde_count * entry_size could wrap around to a small
positive value.

This would bypass the bounds check against sec->eh_frame_hdr_end and
potentially allow the kernel to trust an arbitrarily large fde_count,
leading to out-of-bounds kernel memory reads when performing binary searches.

Would it be safer to use check_mul_overflow() and check_add_overflow() for
these bounds calculations?
quoted
+	}
+
+end:
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/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help