ELF loader should validate before load by bentheredonethat · Pull Request #687 · OpenAMP/open-amp

bentheredonethat · 2026-06-01T15:29:46Z

OpenAMP’s ELF loader should reject malformed firmware images before using ELF header metadata to size allocations, copy header tables, or look up section names.

OpenAMP’s remoteproc ELF loader trusts several ELF header and section-header fields while parsing firmware images. These fields control program-header and section-header table sizes, table
offsets, segment copy ranges, and section string-table lookups.

The main issue is in the header-table loading path. The loader computes program and section table sizes from firmware-controlled values such as e_phnum * e_phentsize and e_shnum * e_shentsize.
Those computed sizes are then used for allocation and memcpy(), while later code walks the copied buffers as native Elf32_Phdr, Elf64_Phdr, Elf32_Shdr, or Elf64_Shdr arrays. If the count, entry
size, or offset metadata is malformed, the allocation/copy size can diverge from the way the loader later indexes the table.

The range checks around these copies also rely on offset-plus-length arithmetic. With malformed offsets or sizes, unchecked addition can wrap and make an invalid range appear valid. Similar range
assumptions apply when loading the section string table and when the loader asks the backing image store for the next chunk of firmware data.

Section-name lookup has a separate parser-side read issue. The loader searches for sections such as .resource_table by using each section header’s sh_name as an offset into the loaded section
string table. Because sh_name is firmware-controlled, the loader must not form or compare name_table + sh_name unless the offset is within the loaded string table and the referenced string is NUL-
terminated before the end of that table.

The fix makes the ELF loader fail closed on malformed metadata:

add portable checked arithmetic helpers for size_t multiplication, addition, and image-chunk range validation;
validate ELF header shape before loading tables, including ELF class, ELF header size, program-header entry size, section-header entry size, and section string-table index;
use checked multiplication for program-header and section-header table sizes;
replace wrapping offset-plus-length checks with overflow-safe range validation;
validate the section string-table range before copying it;
store the loaded section string-table size in the ELF image state;
bound section-name lookup by checking sh_name and comparing only within the loaded string table;
avoid wrapping arithmetic in the remoteproc caller’s non-seekable offset comparison.

With these changes, valid firmware images continue to load normally, while malformed images with inconsistent table metadata, overflowing ranges, invalid entry sizes, or out-of-bounds section
names are rejected before allocation, copy, or table lookup proceeds.

edmooring · 2026-06-17T05:47:43Z

+static int elf_validate_header(const void *elf_info)
+{
+	if (elf_is_64(elf_info) == 0) {
+		const Elf32_Ehdr *ehdr = elf_info;


I think we can ignore the compliance check failures for the camel case ELF header names. They are defined in the ELF spec this way.

edmooring · 2026-06-17T05:59:25Z

 #include <openamp/elf_loader.h>
 #include <openamp/remoteproc.h>

+static int elf_mul_size(size_t a, size_t b, size_t *out)


This applies to all the newly defined functions. They need Doxygen comments to describe them.

edmooring · 2026-06-17T06:03:25Z

+	return 0;
+}
+
+static int elf_range_in_chunk(size_t chunk_offset, size_t chunk_len,


This function really needs the Doxygen comment to describe its return values. I think it returns zero on success, -RPROC_EINVAL for overflows, and 1 for just wrong values. If this is the case, then why can't it just return -RPROC_EINVAL for all error cases? This would simplify the code flow for users of this function.

edmooring · 2026-06-17T06:03:58Z

 	}
 }

+static int elf_validate_header(const void *elf_info)


Doxygen comments would be nice.

edmooring · 2026-06-17T06:09:44Z

+						    shdrs_size);
+		if (range_in_chunk < 0)
+			return range_in_chunk;
+		if (!range_in_chunk) {


What happens if elf_range_in_chunk() returns 1? I think that means that the inputs are wrong, but don't overflow.

edmooring · 2026-06-17T06:23:43Z

+						    shstrtab_size);
+		if (range_in_chunk < 0)
+			return range_in_chunk;
+		if (!range_in_chunk) {


Again, what happens if elf_range_in_chunk() returns 1? Some values don't get assigned.

Add local helpers in the ELF loader for checked size_t multiplication, addition, and image chunk range validation. The ELF loader derives allocation sizes and copy ranges from firmware header fields. Centralizing the overflow checks keeps later validation portable across embedded toolchains and avoids relying on compiler builtins. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Reject malformed ELF headers before using program or section table metadata from the firmware image. Validate the ELF class, ELF header size, program header entry size, section header entry size, and section string-table index. This prevents the loader from allocating or indexing native header arrays using entry sizes that do not match the structures used by OpenAMP. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Use checked multiplication when computing ELF program and section header table sizes from e_phnum/e_phentsize and e_shnum/e_shentsize. Also replace wrapping offset-plus-length range checks with overflow-safe image chunk validation before allocation and memcpy. Malformed firmware images with impossible table sizes now fail with -RPROC_EINVAL. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Use overflow-safe range validation before copying the ELF section string table from the firmware image. The section string-table offset and size are read from untrusted section headers. Validate that the full table is present in the current image chunk without relying on wrapping offset arithmetic. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Store the loaded section string-table size in the ELF image information for both ELF32 and ELF64 images. Keeping the size alongside the string-table pointer allows later section name lookups to validate sh_name offsets against the actual loaded table bounds. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Make ELF section-name lookup validate sh_name before reading from the loaded section string table. Skip malformed section names whose sh_name offset is outside the table or whose string is not NUL-terminated within the remaining table bytes. Use bounded comparison for valid candidates so .resource_table lookup cannot read past the loaded string table. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

Replace the non-seekable loader offset comparison that used offset + len with an overflow-safe equivalent. This preserves existing behavior for normal ranges while avoiding a wraparound case when deciding whether required image data is contiguous with the current chunk. Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>

bentheredonethat · 2026-06-17T17:29:10Z

Hi @edmooring

Addressed in the updated series:

For the ELF type-name compliance findings, I left the Elf32_Ehdr, Elf64_Ehdr, and related ELF names unchanged.

Agreed that these come from the ELF spec and should not be renamed just to satisfy the camel-case check.

For the newly added helper functions, I added Doxygen comments. In particular, elf_range_in_chunk() now documents its return values explicitly:

1: the requested range is contained in the current image chunk
0: the requested range is not contained in the current image chunk
-RPROC_EINVAL: overflow occurred while calculating the checked range endpoints

So 0 is not the success case for this helper. It is the normal streaming-loader path where the range is not present in
the current chunk, and the caller should set *noffset / *nlen and request that range. Returning -RPROC_EINVAL for that case would incorrectly turn a valid “load another chunk” condition into a hard failure.

I also renamed the local variables at the call sites from range_in_chunk to range_loaded to make the control flow clearer. If elf_range_in_chunk() returns 1, the requested table/string table is already present in img_data, so the code intentionally falls through and proceeds to copy from the current chunk. In that path, *noffset and *nlen do ot need to be assigned because no additional chunk is being requested.

If it returns 0, the code assigns *noffset / *nlen and returns the current load state so the caller can fetch the missing range.

arnopo

minor comments .
@bentheredonethat: Did you try to verify with an IA if some other overflows are detected ?

arnopo · 2026-06-19T08:54:40Z

+
+	*out = a + b;
+	return 0;
+}


what about moving tes function in utilities.c ?
renaming themsafe/add_multor mult/add_overflow ( or something else)

ok will fix

arnopo · 2026-06-19T09:09:05Z

+ * @param chunk_offset	Offset of the image chunk from the start of the image.
+ * @param chunk_len	Length of the image chunk.
+ * @param range_offset	Offset of the range from the start of the image.
+ * @param range_len	Length of the range.


Not clear to me what you name range and image chunk.
Here what you want to check is that a is in a range , right?
Could be also a more generic utilities that also clarify the usage

ok will fix - and make more generic utilities helper

arnopo · 2026-06-19T09:09:07Z

+ * @param range_len	Length of the range.
+ *
+ * @return 1 if the range is contained within the chunk, 0 if the range is
+ *	   not contained within the chunk, otherwise -RPROC_EINVAL.


I would return 0 if Ok and an error otherwize. or return a boolean renaming the function in cluding `is' in the name

ok will fix

arnopo · 2026-06-19T09:12:59Z

+ */
+static int elf_validate_header(const void *elf_info)
+{
+	if (elf_is_64(elf_info) == 0) {


i would inverse the logic for readability

if (elf_is_64(elf_info) ) { const Elf64_Ehdr *ehdr = elf_info; [...] } else { const Elf32_Ehdr *ehdr = elf_info; }

ok will fix

arnopo · 2026-06-19T09:13:53Z

+		if (ehdr->e_shnum != 0 &&
+		    ehdr->e_shentsize != sizeof(Elf32_Shdr))
+			return -RPROC_EINVAL;
+		if (ehdr->e_shnum != 0 && ehdr->e_shstrndx >= ehdr->e_shnum)


this one seems common to both format

ok will fix

arnopo · 2026-06-19T09:20:05Z

 			}
 			memcpy(*img_info, img_data, tmpsize);
+			if (elf_validate_header(*img_info) < 0)
+				return -RPROC_EINVAL;


directly return the elf_validate_header result

ok will fix

arnopo · 2026-06-19T09:21:42Z

+		if (elf_mul_size(elf_phnum(*img_info),
+				 elf_phentsize(*img_info),
+				 &phdrs_size) < 0)
+			return -RPROC_EINVAL;


same here return elf_mul_size result

ok will fix

arnopo · 2026-06-19T09:26:26Z

+						  shdrs_size);
+		if (range_loaded < 0)
+			return range_loaded;
+		if (!range_loaded) {


my coment on elf_range_in_chunk return should simplify this code

ok will fix

arnopo · 2026-06-19T09:33:19Z

+		return 0;
+
+	candidate = name_table + sh_name;
+	return memcmp(name, candidate, name_len + 1) == 0;


You return a boolean here and a int otherwise .
I suggest to return a boolean for this function

ok will fix

bentheredonethat · 2026-06-24T13:43:07Z

minor comments . @bentheredonethat: Did you try to verify with an IA if some other overflows are detected ?

@arnopo can you please clarify what is meant by this ? i did some impact analysys by passing in overflowing values

arnopo · 2026-06-26T08:32:10Z

can you please clarify what is meant by this ? i did some impact analysys by passing in overflowing values

My point behind my question is the validation part. Could you share test you performed.
I wonder how we can ensure that we address all the vulnerability reported in https://www.sentinelone.com/vulnerability-database/cve-2026-37540/

arnopo requested review from arnopo, edmooring and tnmysh June 2, 2026 09:07

edmooring requested changes Jun 17, 2026

View reviewed changes

bentheredonethat added 7 commits June 17, 2026 09:41

bentheredonethat force-pushed the amd-elf-load-fixup-june-1-2026 branch from 064b04a to bbf3dfa Compare June 17, 2026 17:29

arnopo requested changes Jun 19, 2026

View reviewed changes

arnopo linked an issue Jun 26, 2026 that may be closed by this pull request

CVE-2026-37540 report: integer overflow vulnerability on ELF Loader #688

Open

arnopo added this to the Release V2026.10 milestone Jun 26, 2026

Uh oh!

Conversation

bentheredonethat commented Jun 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bentheredonethat commented Jun 17, 2026

Uh oh!

arnopo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bentheredonethat commented Jun 24, 2026

Uh oh!

arnopo commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arnopo left a comment •

edited

Loading