lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200727230801.3468620-9-nivedita@alum.mit.edu>
Date:   Mon, 27 Jul 2020 19:08:01 -0400
From:   Arvind Sankar <nivedita@...m.mit.edu>
To:     Kees Cook <keescook@...omium.org>, x86@...nel.org
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH v2 8/8] x86/kaslr: Don't use 64-bit mem_vector for 32-bit kernel

Commit
  f28442497b5c ("x86/boot: Fix KASLR and memmap= collision")
converted mem_vector type to use 64-bit on the 32-bit kernel as well,
based on Thomas's review [0]. However:
- the code still doesn't consistently use 64-bit types. For instance,
  mem_avoid_overlap uses 32-bit types when checking for overlaps.  This
  is actually okay, as the passed in memory range will have been clipped
  to below 4G, but it is difficult to reason about the safety of the
  code.
- KASLR on 32-bit can't use memory above 4G anyway, so it's pointless
  to keep track of ranges above 4G.

Switch the type back to unsigned long, and use a helper function to clip
regions to 4G on 32-bit, when storing mem_avoid, immovable_mem, EFI,
E820 and command-line memory regions.

[0] https://lore.kernel.org/linux-nvdimm/alpine.DEB.2.20.1701111246400.3579@nanos/

Signed-off-by: Arvind Sankar <nivedita@...m.mit.edu>
---
 arch/x86/boot/compressed/acpi.c  |  7 +++----
 arch/x86/boot/compressed/kaslr.c | 25 ++++++++++---------------
 arch/x86/boot/compressed/misc.h  | 19 +++++++++++++++++--
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 8bcbcee54aa1..d09e0ec5637e 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -106,7 +106,7 @@ static acpi_physical_address kexec_get_rsdp_addr(void)
 	}
 
 	/* Get systab from boot params. */
-	systab = (efi_system_table_64_t *) (ei->efi_systab | ((__u64)ei->efi_systab_hi << 32));
+	systab = (efi_system_table_64_t *) (ei->efi_systab | ((u64)ei->efi_systab_hi << 32));
 	if (!systab)
 		error("EFI system table not found in kexec boot_params.");
 
@@ -139,7 +139,7 @@ static acpi_physical_address efi_get_rsdp_addr(void)
 
 	/* Get systab from boot params. */
 #ifdef CONFIG_X86_64
-	systab = ei->efi_systab | ((__u64)ei->efi_systab_hi << 32);
+	systab = ei->efi_systab | ((u64)ei->efi_systab_hi << 32);
 #else
 	if (ei->efi_systab_hi || ei->efi_memmap_hi) {
 		debug_putstr("Error getting RSDP address: EFI system table located above 4GB.\n");
@@ -404,8 +404,7 @@ int count_immovable_mem_regions(void)
 
 			ma = (struct acpi_srat_mem_affinity *)sub_table;
 			if (!(ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && ma->length) {
-				immovable_mem[num].start = ma->base_address;
-				immovable_mem[num].size = ma->length;
+				immovable_mem[num] = mem_vector_ctor(ma->base_address, ma->length);
 				num++;
 			}
 
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index eca2acc65e2a..8c44041ae9cb 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -98,7 +98,7 @@ static bool memmap_too_large;
  * Store memory limit: MAXMEM on 64-bit and KERNEL_IMAGE_SIZE on 32-bit.
  * It may be reduced by "mem=nn[KMG]" or "memmap=nn[KMG]" command line options.
  */
-static unsigned long long mem_limit;
+static unsigned long mem_limit;
 
 /* Number of immovable memory regions */
 static int num_immovable_mem;
@@ -141,8 +141,7 @@ enum parse_mode {
 };
 
 static int
-parse_memmap(char *p, unsigned long long *start, unsigned long long *size,
-		enum parse_mode mode)
+parse_memmap(char *p, u64 *start, u64 *size, enum parse_mode mode)
 {
 	char *oldp;
 
@@ -172,7 +171,7 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size,
 			 */
 			*size = 0;
 		} else {
-			unsigned long long flags;
+			u64 flags;
 
 			/*
 			 * efi_fake_mem=nn@ss:attr the attr specifies
@@ -211,7 +210,7 @@ static void mem_avoid_memmap(enum parse_mode mode, char *str)
 
 	while (str && (i < MAX_MEMMAP_REGIONS)) {
 		int rc;
-		unsigned long long start, size;
+		u64 start, size;
 		char *k = strchr(str, ',');
 
 		if (k)
@@ -230,8 +229,7 @@ static void mem_avoid_memmap(enum parse_mode mode, char *str)
 			continue;
 		}
 
-		mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].start = start;
-		mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].size = size;
+		mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i] = mem_vector_ctor(start, size);
 		i++;
 	}
 
@@ -422,8 +420,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	initrd_start |= boot_params->hdr.ramdisk_image;
 	initrd_size  = (u64)boot_params->ext_ramdisk_size << 32;
 	initrd_size |= boot_params->hdr.ramdisk_size;
-	mem_avoid[MEM_AVOID_INITRD].start = initrd_start;
-	mem_avoid[MEM_AVOID_INITRD].size = initrd_size;
+	mem_avoid[MEM_AVOID_INITRD] = mem_vector_ctor(initrd_start, initrd_size);
 	/* No need to set mapping for initrd, it will be handled in VO. */
 
 	/* Avoid kernel command line. */
@@ -673,7 +670,7 @@ static bool process_mem_region(struct mem_vector *region,
 	 * immovable memory and @region.
 	 */
 	for (i = 0; i < num_immovable_mem; i++) {
-		unsigned long long start, end, entry_end, region_end;
+		unsigned long start, end, entry_end, region_end;
 		struct mem_vector entry;
 
 		if (!mem_overlaps(region, &immovable_mem[i]))
@@ -728,7 +725,7 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
 	}
 	pmap =  e->efi_memmap;
 #else
-	pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
+	pmap = (e->efi_memmap | ((u64)e->efi_memmap_hi << 32));
 #endif
 
 	nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
@@ -765,8 +762,7 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
 		    !(md->attribute & EFI_MEMORY_MORE_RELIABLE))
 			continue;
 
-		region.start = md->phys_addr;
-		region.size = md->num_pages << EFI_PAGE_SHIFT;
+		region = mem_vector_ctor(md->phys_addr, md->num_pages << EFI_PAGE_SHIFT);
 		if (process_mem_region(&region, minimum, image_size))
 			break;
 	}
@@ -793,8 +789,7 @@ static void process_e820_entries(unsigned long minimum,
 		/* Skip non-RAM entries. */
 		if (entry->type != E820_TYPE_RAM)
 			continue;
-		region.start = entry->addr;
-		region.size = entry->size;
+		region = mem_vector_ctor(entry->addr, entry->size);
 		if (process_mem_region(&region, minimum, image_size))
 			break;
 	}
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 726e264410ff..fd7c49ab0f02 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -70,10 +70,25 @@ int cmdline_find_option(const char *option, char *buffer, int bufsize);
 int cmdline_find_option_bool(const char *option);
 
 struct mem_vector {
-	unsigned long long start;
-	unsigned long long size;
+	unsigned long start;
+	unsigned long size;
 };
 
+static inline
+struct mem_vector mem_vector_ctor(u64 start, u64 size)
+{
+	u64 end = start + size;
+	struct mem_vector ret;
+
+	start = min_t(u64, start, ULONG_MAX);
+	end   = min_t(u64, end,   ULONG_MAX);
+
+	ret.start = (unsigned long)start;
+	ret.size  = (unsigned long)(end - start);
+
+	return ret;
+}
+
 #if CONFIG_RANDOMIZE_BASE
 /* kaslr.c */
 void choose_random_location(unsigned long input,
-- 
2.26.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ