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

On 8/18/2026 5:02 PM, [email protected] wrote:

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

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

>> +		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> 
>> +	case DW_EH_PE_uleb128: {
>> +		unsigned long tmp;
>> +		ret = read_uleb128(&cur, end, &tmp);
>> +		if (ret)
>> +			return ret;
>> +		result += tmp;
>> +		break;
>> +	}

>> +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;
> [ ... ]
>> +		/* 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;

>> +		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?
> 
>> +	}
>> +
>> +end:
> 

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/