Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Apr 2018 22:23:22 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r332420 - in stable/11: stand/common sys/kern
Message-ID:  <201804112223.w3BMNMmV068849@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Wed Apr 11 22:23:22 2018
New Revision: 332420
URL: https://svnweb.freebsd.org/changeset/base/332420

Log:
  MFC 328101,328911: Require SHF_ALLOC for kernel object module sections.
  
  328101:
  Require the SHF_ALLOC flag for program sections from kernel object modules.
  
  ELF object files can contain program sections which are not supposed
  to be loaded into memory (e.g. .comment).  Normally the static linker
  uses these flags to decide which sections are allocated to loadable
  program segments in ELF binaries and shared objects (including kernels
  on all architectures and kernel modules on architectures other than
  amd64).
  
  Mapping ELF object files (such as amd64 kernel modules) into memory
  directly is a bit of a grey area.  ELF object files are intended to be
  used as inputs to the static linker.  As a result, there is not a
  standardized definition for what the memory layout of an ELF object
  should be (none of the section headers have valid virtual memory
  addresses for example).
  
  The kernel and loader were not checking the SHF_ALLOC flag but loading
  any program sections with certain types such as SHT_PROGBITS.  As a
  result, the kernel and loader would load into RAM some sections that
  weren't marked with SHF_ALLOC such as .comment that are not loaded
  into RAM for kernel modules on other architectures (which are
  implemented as ELF shared objects).  Aside from possibly requiring
  slightly more RAM to hold a kernel module this does not affect runtime
  correctness as the kernel relocates symbols based on the layout it
  uses.
  
  Debuggers such as gdb and lldb do not extract symbol tables from a
  running process or kernel.  Instead, they replicate the memory layout
  of ELF executables and shared objects and use that to construct their
  own symbol tables.  For executables and shared objects this works
  fine.  For ELF objects the current logic in kgdb (and probably lldb
  based on a simple reading) assumes that only sections with SHF_ALLOC
  are memory resident when constructing a memory layout.  If the
  debugger constructs a different memory layout than the kernel, then it
  will compute different addresses for symbols causing symbols in the
  debugger to appear to have the wrong values (though the kernel itself
  is working fine).  The current port of mdb does not check SHF_ALLOC as
  it replicates the kernel's logic in its existing kernel support.
  
  The bfd linker sorts the sections in ELF object files such that all of
  the allocated sections (sections with SHF_ALLOCATED) are placed first
  followed by unallocated sections.  As a result, when kgdb composed a
  memory layout using only the allocated sections, this layout happened
  to match the layout used by the kernel and loader.  The lld linker
  does not sort the sections in ELF object files and mixed allocated and
  unallocated sections.  This resulted in kgdb composing a different
  memory layout than the kernel and loader.
  
  We could either patch kgdb (and possibly in the future lldb) to use
  custom handling when generating memory layouts for kernel modules that
  are ELF objects, or we could change the kernel and loader to check
  SHF_ALLOCATED.  I chose the latter as I feel we shouldn't be loading
  things into RAM that the module won't use.  This should mostly be a
  NOP when linking with bfd but will allow the existing kgdb to work
  with amd64 kernel modules linked with lld.
  
  Note that we only require SHF_ALLOC for "program" sections for types
  like SHT_PROGBITS and SHT_NOBITS.  Other section types such as symbol
  tables, string tables, and relocations must also be loaded and are not
  marked with SHF_ALLOC.
  
  328911:
  Ignore relocation tables for non-memory-resident sections.
  
  As a followup to r328101, ignore relocation tables for ELF object
  sections that are not memory resident.  For modules loaded by the
  loader, ignore relocation tables whose associated section was not
  loaded by the loader (sh_addr is zero).  For modules loaded at runtime
  via kldload(2), ignore relocation tables whose associated section is
  not marked with SHF_ALLOC.

Modified:
  stable/11/stand/common/load_elf_obj.c
  stable/11/sys/kern/link_elf_obj.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/stand/common/load_elf_obj.c
==============================================================================
--- stable/11/stand/common/load_elf_obj.c	Wed Apr 11 21:41:59 2018	(r332419)
+++ stable/11/stand/common/load_elf_obj.c	Wed Apr 11 22:23:22 2018	(r332420)
@@ -224,6 +224,8 @@ __elfN(obj_loadimage)(struct preloaded_file *fp, elf_f
 #if defined(__i386__) || defined(__amd64__)
 		case SHT_X86_64_UNWIND:
 #endif
+			if ((shdr[i].sh_flags & SHF_ALLOC) == 0)
+				break;
 			lastaddr = roundup(lastaddr, shdr[i].sh_addralign);
 			shdr[i].sh_addr = (Elf_Addr)lastaddr;
 			lastaddr += shdr[i].sh_size;
@@ -280,6 +282,8 @@ __elfN(obj_loadimage)(struct preloaded_file *fp, elf_f
 		switch (shdr[i].sh_type) {
 		case SHT_REL:
 		case SHT_RELA:
+			if ((shdr[shdr[i].sh_info].sh_flags & SHF_ALLOC) == 0)
+				break;
 			lastaddr = roundup(lastaddr, shdr[i].sh_addralign);
 			shdr[i].sh_addr = (Elf_Addr)lastaddr;
 			lastaddr += shdr[i].sh_size;

Modified: stable/11/sys/kern/link_elf_obj.c
==============================================================================
--- stable/11/sys/kern/link_elf_obj.c	Wed Apr 11 21:41:59 2018	(r332419)
+++ stable/11/sys/kern/link_elf_obj.c	Wed Apr 11 22:23:22 2018	(r332420)
@@ -260,6 +260,9 @@ link_elf_link_preload(linker_class_t cls, const char *
 #ifdef __amd64__
 		case SHT_X86_64_UNWIND:
 #endif
+			/* Ignore sections not loaded by the loader. */
+			if (shdr[i].sh_addr == 0)
+				break;
 			ef->nprogtab++;
 			break;
 		case SHT_SYMTAB:
@@ -267,9 +270,17 @@ link_elf_link_preload(linker_class_t cls, const char *
 			symstrindex = shdr[i].sh_link;
 			break;
 		case SHT_REL:
+			/*
+			 * Ignore relocation tables for sections not
+			 * loaded by the loader.
+			 */
+			if (shdr[shdr[i].sh_info].sh_addr == 0)
+				break;
 			ef->nreltab++;
 			break;
 		case SHT_RELA:
+			if (shdr[shdr[i].sh_info].sh_addr == 0)
+				break;
 			ef->nrelatab++;
 			break;
 		}
@@ -333,6 +344,8 @@ link_elf_link_preload(linker_class_t cls, const char *
 #ifdef __amd64__
 		case SHT_X86_64_UNWIND:
 #endif
+			if (shdr[i].sh_addr == 0)
+				break;
 			ef->progtab[pb].addr = (void *)shdr[i].sh_addr;
 			if (shdr[i].sh_type == SHT_PROGBITS)
 				ef->progtab[pb].name = "<<PROGBITS>>";
@@ -391,12 +404,16 @@ link_elf_link_preload(linker_class_t cls, const char *
 			pb++;
 			break;
 		case SHT_REL:
+			if (shdr[shdr[i].sh_info].sh_addr == 0)
+				break;
 			ef->reltab[rl].rel = (Elf_Rel *)shdr[i].sh_addr;
 			ef->reltab[rl].nrel = shdr[i].sh_size / sizeof(Elf_Rel);
 			ef->reltab[rl].sec = shdr[i].sh_info;
 			rl++;
 			break;
 		case SHT_RELA:
+			if (shdr[shdr[i].sh_info].sh_addr == 0)
+				break;
 			ef->relatab[ra].rela = (Elf_Rela *)shdr[i].sh_addr;
 			ef->relatab[ra].nrela =
 			    shdr[i].sh_size / sizeof(Elf_Rela);
@@ -599,6 +616,8 @@ link_elf_load_file(linker_class_t cls, const char *fil
 #ifdef __amd64__
 		case SHT_X86_64_UNWIND:
 #endif
+			if ((shdr[i].sh_flags & SHF_ALLOC) == 0)
+				break;
 			ef->nprogtab++;
 			break;
 		case SHT_SYMTAB:
@@ -607,9 +626,17 @@ link_elf_load_file(linker_class_t cls, const char *fil
 			symstrindex = shdr[i].sh_link;
 			break;
 		case SHT_REL:
+			/*
+			 * Ignore relocation tables for unallocated
+			 * sections.
+			 */
+			if ((shdr[shdr[i].sh_info].sh_flags & SHF_ALLOC) == 0)
+				break;
 			ef->nreltab++;
 			break;
 		case SHT_RELA:
+			if ((shdr[shdr[i].sh_info].sh_flags & SHF_ALLOC) == 0)
+				break;
 			ef->nrelatab++;
 			break;
 		case SHT_STRTAB:
@@ -714,6 +741,8 @@ link_elf_load_file(linker_class_t cls, const char *fil
 #ifdef __amd64__
 		case SHT_X86_64_UNWIND:
 #endif
+			if ((shdr[i].sh_flags & SHF_ALLOC) == 0)
+				break;
 			alignmask = shdr[i].sh_addralign - 1;
 			mapsize += alignmask;
 			mapsize &= ~alignmask;
@@ -784,6 +813,8 @@ link_elf_load_file(linker_class_t cls, const char *fil
 #ifdef __amd64__
 		case SHT_X86_64_UNWIND:
 #endif
+			if ((shdr[i].sh_flags & SHF_ALLOC) == 0)
+				break;
 			alignmask = shdr[i].sh_addralign - 1;
 			mapbase += alignmask;
 			mapbase &= ~alignmask;
@@ -863,6 +894,8 @@ link_elf_load_file(linker_class_t cls, const char *fil
 			pb++;
 			break;
 		case SHT_REL:
+			if ((shdr[shdr[i].sh_info].sh_flags & SHF_ALLOC) == 0)
+				break;
 			ef->reltab[rl].rel = malloc(shdr[i].sh_size, M_LINKER,
 			    M_WAITOK);
 			ef->reltab[rl].nrel = shdr[i].sh_size / sizeof(Elf_Rel);
@@ -881,6 +914,8 @@ link_elf_load_file(linker_class_t cls, const char *fil
 			rl++;
 			break;
 		case SHT_RELA:
+			if ((shdr[shdr[i].sh_info].sh_flags & SHF_ALLOC) == 0)
+				break;
 			ef->relatab[ra].rela = malloc(shdr[i].sh_size, M_LINKER,
 			    M_WAITOK);
 			ef->relatab[ra].nrela =



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201804112223.w3BMNMmV068849>