elf_loader: resolve outstanding validation fixme

Signed-off-by: Carl van Schaik <quic_cvanscha@quicinc.com>
shqli · Oct 27, 2021 · 6dded12 · 6dded12
1 parent c667ce8
commit 6dded12
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 19 deletions.
diff --git a/hyp/interfaces/elf/include/elf_loader.h b/hyp/interfaces/elf/include/elf_loader.h
@@ -20,4 +20,4 @@ elf_get_phdr(void *elf_file, count_t index);
 
 // Load the ELF file to its physical address as per its program headers
 error_t
-elf_load_phys(void *elf_file, paddr_t phys_offset);
+elf_load_phys(void *elf_file, size_t elf_max_size, paddr_t phys_base);
diff --git a/hyp/misc/elf/src/elf_loader.c b/hyp/misc/elf/src/elf_loader.c
@@ -124,8 +124,9 @@ elf_get_phdr(void *elf_file, count_t index)
 }
 
 error_t
-elf_load_phys(void *elf_file, paddr_t phys_offset)
+elf_load_phys(void *elf_file, size_t elf_max_size, paddr_t phys_base)
 {
+	error_t	  ret;
 	index_t	  i;
 	uintptr_t elf_base = (uintptr_t)elf_file;
 	Elf_Ehdr *ehdr	   = (Elf_Ehdr *)(uintptr_t)elf_file;
@@ -143,11 +144,31 @@ elf_load_phys(void *elf_file, paddr_t phys_offset)
 			continue;
 		}
 
-		// FIXME: validate addresses target useable HYP memory
+		if (cur_phdr->p_filesz > cur_phdr->p_memsz) {
+			ret = ERROR_ARGUMENT_SIZE;
+			goto out;
+		}
+
+		if (util_add_overflows(cur_phdr->p_paddr, cur_phdr->p_memsz) ||
+		    util_add_overflows(phys_base,
+				       cur_phdr->p_paddr + cur_phdr->p_memsz) ||
+		    ((cur_phdr->p_offset + cur_phdr->p_filesz) >
+		     elf_max_size)) {
+			ret = ERROR_ARGUMENT_SIZE;
+			goto out;
+		}
+
+		if (util_add_overflows(cur_phdr->p_offset, cur_phdr->p_memsz) ||
+		    util_add_overflows(elf_base, cur_phdr->p_offset +
+							 cur_phdr->p_memsz)) {
+			ret = ERROR_ARGUMENT_SIZE;
+			goto out;
+		}
+
 		uintptr_t seg_base = elf_base + cur_phdr->p_offset;
-		paddr_t	  seg_dest = cur_phdr->p_paddr + phys_offset;
+		paddr_t	  seg_dest = phys_base + cur_phdr->p_paddr;
 
-		error_t ret;
+		error_t err;
 
 		paddr_t map_base = util_balign_down(
 			seg_dest, (paddr_t)PGTABLE_HYP_PAGE_SIZE);
@@ -157,24 +178,25 @@ elf_load_phys(void *elf_file, paddr_t phys_offset)
 			map_base;
 
 		// FIXME: should we use phys_map and phys_access eventually?
-		ret = hyp_aspace_map_direct(map_base, map_size,
+		err = hyp_aspace_map_direct(map_base, map_size,
 					    PGTABLE_ACCESS_RW,
 					    PGTABLE_HYP_MEMTYPE_WRITETHROUGH,
 					    VMSA_SHAREABILITY_INNER_SHAREABLE);
-		assert(ret == OK);
+		assert(err == OK);
 
 		// copy elf segment data
 		memcpy((char *)seg_dest, (char *)seg_base, cur_phdr->p_filesz);
-		if (cur_phdr->p_memsz > cur_phdr->p_filesz) {
-			// zero bss
-			memset((char *)(seg_dest + cur_phdr->p_filesz), 0,
-			       cur_phdr->p_memsz - cur_phdr->p_filesz);
-		}
+		// zero bss
+		memset((char *)(seg_dest + cur_phdr->p_filesz), 0,
+		       cur_phdr->p_memsz - cur_phdr->p_filesz);
 
 		LOG(DEBUG, INFO, "Elf copied from {:#x} to {:#x} - size {:#x}",
 		    seg_base, seg_dest, cur_phdr->p_filesz);
-		ret = hyp_aspace_unmap_direct(map_base, map_size);
-		assert(ret == OK);
+		err = hyp_aspace_unmap_direct(map_base, map_size);
+		assert(err == OK);
 	}
-	return OK;
+
+	ret = OK;
+out:
+	return ret;
 }
diff --git a/hyp/vm/rootvm_package/src/package.c b/hyp/vm/rootvm_package/src/package.c
@@ -80,8 +80,9 @@ create_memextent(partition_t *root_partition, cspace_t *root_cspace,
 }
 
 static paddr_t
-rootvm_package_load_elf(void *elf, addrspace_t *addrspace, vmaddr_t ipa_base,
-			paddr_t phys_offset, memextent_t *me_rm)
+rootvm_package_load_elf(void *elf, size_t elf_max_size, addrspace_t *addrspace,
+			vmaddr_t ipa_base, paddr_t phys_offset,
+			memextent_t *me_rm)
 {
 	error_t err;
 	count_t i;
@@ -147,7 +148,7 @@ rootvm_package_load_elf(void *elf, addrspace_t *addrspace, vmaddr_t ipa_base,
 		panic("ELF segment out of range");
 	}
 
-	err = elf_load_phys(elf, phys_offset);
+	err = elf_load_phys(elf, elf_max_size, phys_offset);
 	if (err != OK) {
 		panic("Error loading ELF");
 	}
@@ -239,6 +240,13 @@ rootvm_package_handle_rootvm_init(partition_t *root_partition,
 			void *elf =
 				(void *)(map_base + pkg_hdr->list[i].offset);
 
+			if (pkg_hdr->list[i].offset > map_size) {
+				panic("ELF out of valid region");
+			}
+
+			size_t elf_max_size =
+				map_size - pkg_hdr->list[i].offset;
+
 			if (!elf_valid(elf)) {
 				panic("Invalid package ELF");
 			}
@@ -254,7 +262,8 @@ rootvm_package_handle_rootvm_init(partition_t *root_partition,
 				app_ipa = ipa + offset;
 			}
 
-			load_next = rootvm_package_load_elf(elf, addrspace, ipa,
+			load_next = rootvm_package_load_elf(elf, elf_max_size,
+							    addrspace, ipa,
 							    load_next, me);
 			break;
 		case ROOTVM_PACKAGE_IMAGE_TYPE_UNKNOWN: