From 51d6d0952bcbf7971ece8d5f142ae296a9fd02e9 Mon Sep 17 00:00:00 2001 From: Bill Paul Date: Wed, 26 Oct 2005 18:46:27 +0000 Subject: [PATCH] Clean up and apply the fix for PR 83477. The calculation for locating the start of the section headers has to take into account the fact that the image_nt_header is really variable sized. It happens that the existing calculation is correct for _most_ production binaries produced by the Windows DDK, but if we get a binary with oddball offsets, the PE loader could crash. Changes from the supplied patch are: - We don't really need to use the IMAGE_SIZEOF_NT_HEADER() macro when computing how much of the header to return to callers of pe_get_optional_header(). While it's important to take the variable size of the header into account in other calculations, we never actually look at anything outside the non-variable portion of the header. This saves callers from having to allocate a variable sized buffer off the heap (I purposely tried to avoid using malloc() in subr_pe.c to make it easier to compile in both the -D_KERNEL and !-D_KERNEL case), and since we're copying into a buffer on the stack, we always have to copy the same amount of data or else we'll trash the stack something fierce. - We need to get offsetof() in the !-D_KERNEL case. - ndiscvt.c needs the IMAGE_FIRST_SECTION() macro too, since it does a little bit of section pre-processing. PR: kern/83477 --- sys/compat/ndis/pe_var.h | 9 +++++++++ sys/compat/ndis/subr_pe.c | 20 +++++++++++++------- usr.sbin/ndiscvt/ndiscvt.c | 4 ++-- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/sys/compat/ndis/pe_var.h b/sys/compat/ndis/pe_var.h index 1717f32dc8fd..e778b5805532 100644 --- a/sys/compat/ndis/pe_var.h +++ b/sys/compat/ndis/pe_var.h @@ -214,6 +214,10 @@ struct image_nt_header { typedef struct image_nt_header image_nt_header; +#define IMAGE_SIZEOF_NT_HEADER(nthdr) \ + (offsetof(image_nt_header, inh_optionalhdr) + \ + ((image_nt_header *)(nthdr))->inh_filehdr.ifh_optionalhdrlen) + /* Directory Entries */ #define IMAGE_DIRECTORY_ENTRY_EXPORT 0 /* Export Directory */ @@ -281,6 +285,11 @@ typedef struct image_section_header image_section_header; #define IMAGE_SIZEOF_SECTION_HEADER 40 +#define IMAGE_FIRST_SECTION(nthdr) \ + ((image_section_header *)((vm_offset_t)(nthdr) + \ + offsetof(image_nt_header, inh_optionalhdr) + \ + ((image_nt_header *)(nthdr))->inh_filehdr.ifh_optionalhdrlen)) + /* * Import format */ diff --git a/sys/compat/ndis/subr_pe.c b/sys/compat/ndis/subr_pe.c index a3f5071ebe16..ee8c0fe0c9f0 100644 --- a/sys/compat/ndis/subr_pe.c +++ b/sys/compat/ndis/subr_pe.c @@ -57,6 +57,7 @@ extern int ndis_strncasecmp(const char *, const char *, size_t); #define strncasecmp(a, b, c) ndis_strncasecmp(a, b, c) #else #include +#include #include #include #include @@ -142,7 +143,7 @@ pe_get_optional_header(imgbase, hdr) nt_hdr = (image_nt_header *)(imgbase + dos_hdr->idh_lfanew); bcopy ((char *)&nt_hdr->inh_optionalhdr, (char *)hdr, - sizeof(image_optional_header)); + nt_hdr->inh_filehdr.ifh_optionalhdrlen); return(0); } @@ -169,6 +170,14 @@ pe_get_file_header(imgbase, hdr) dos_hdr = (image_dos_header *)imgbase; nt_hdr = (image_nt_header *)(imgbase + dos_hdr->idh_lfanew); + /* + * Note: the size of the nt_header is variable since it + * can contain optional fields, as indicated by ifh_optionalhdrlen. + * However it happens we're only interested in fields in the + * non-variant portion of the nt_header structure, so we don't + * bother copying the optional parts here. + */ + bcopy ((char *)&nt_hdr->inh_filehdr, (char *)hdr, sizeof(image_file_header)); @@ -197,8 +206,7 @@ pe_get_section_header(imgbase, hdr) dos_hdr = (image_dos_header *)imgbase; nt_hdr = (image_nt_header *)(imgbase + dos_hdr->idh_lfanew); - sect_hdr = (image_section_header *)((vm_offset_t)nt_hdr + - sizeof(image_nt_header)); + sect_hdr = IMAGE_FIRST_SECTION(nt_hdr); bcopy ((char *)sect_hdr, (char *)hdr, sizeof(image_section_header)); @@ -280,8 +288,7 @@ pe_translate_addr(imgbase, rva) dos_hdr = (image_dos_header *)imgbase; nt_hdr = (image_nt_header *)(imgbase + dos_hdr->idh_lfanew); - sect_hdr = (image_section_header *)((vm_offset_t)nt_hdr + - sizeof(image_nt_header)); + sect_hdr = IMAGE_FIRST_SECTION(nt_hdr); /* * The test here is to see if the RVA falls somewhere @@ -339,8 +346,7 @@ pe_get_section(imgbase, hdr, name) dos_hdr = (image_dos_header *)imgbase; nt_hdr = (image_nt_header *)(imgbase + dos_hdr->idh_lfanew); - sect_hdr = (image_section_header *)((vm_offset_t)nt_hdr + - sizeof(image_nt_header)); + sect_hdr = IMAGE_FIRST_SECTION(nt_hdr); for (i = 0; i < sections; i++) { if (!strcmp ((char *)§_hdr->ish_name, name)) { diff --git a/usr.sbin/ndiscvt/ndiscvt.c b/usr.sbin/ndiscvt/ndiscvt.c index 090f0834f660..80eae836f080 100644 --- a/usr.sbin/ndiscvt/ndiscvt.c +++ b/usr.sbin/ndiscvt/ndiscvt.c @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include #include @@ -88,8 +89,7 @@ extern const char *__progname; #define SET_HDRS(x) \ dos_hdr = (image_dos_header *)x; \ nt_hdr = (image_nt_header *)(x + dos_hdr->idh_lfanew); \ - sect_hdr = (image_section_header *)((vm_offset_t)nt_hdr + \ - sizeof(image_nt_header)); + sect_hdr = IMAGE_FIRST_SECTION(nt_hdr); static int insert_padding(imgbase, imglen)