Today, RealTek sent me a driver to test which had been compiled with

some debug support turned on. It turns out the sections in this driver
binary had relative virtual addresses (RVAs) that were different
from the raw addresses, and it had a .data section where the virtual size
was much larger than the raw size. (Most production binaries produced
with the Microsoft DDK have RVA == PA.)

There's code in the ndiscvt(8) utility that's supposed to handle
the vsize != rsize case, but it turns out it was slightly broken,
and it failed to handle the RVA != RA case at all. Hopefully, this
commit will fix all that.

1 files changed, 43 insertions, 45 deletions

diff --git a/usr.sbin/ndiscvt/ndiscvt.c b/usr.sbin/ndiscvt/ndiscvt.c
index 8c78d54..a24711e 100644
--- a/usr.sbin/ndiscvt/ndiscvt.c
+++ b/usr.sbin/ndiscvt/ndiscvt.c
@@ -54,27 +54,36 @@ static int insert_padding(void **, int *);
 extern const char *__progname;
 
 /*
- * Sections in object code files can be sparse. That is, the
- * section may occupy more space in memory that it does when
- * stored in a disk file. In Windows PE files, each section header
- * has a 'virtual size' and 'raw data size' field. The latter
- * specifies the amount of section data actually stored in the
- * disk file, and the former describes how much space the section
- * should actually occupy in memory. If the vsize is larger than
- * the rsize, we need to allocate some extra storage and fill
- * it with zeros. (Think BSS.)
+ * Sections within Windows PE files are defined using virtual
+ * and physical address offsets and virtual and physical sizes.
+ * The physical values define how the section data is stored in
+ * the executable file while the virtual values describe how the
+ * sections will look once loaded into memory. It happens that
+ * the linker in the Microsoft(r) DDK will tend to generate
+ * binaries where the virtual and physical values are identical,
+ * which means in most cases we can just transfer the file
+ * directly to memory without any fixups. This is not always
+ * the case though, so we have to be prepared to handle files
+ * where the in-memory section layout differs from the disk file
+ * section layout.
  *
- * The typical method of loading an executable file involves
- * reading each segment into memory using the vaddr/vsize from
- * each section header. We try to make a small optimization however
- * and only pad/move segments when it's absolutely necessary, i.e.
- * if the vsize is larger than the rsize. This conserves a little
- * bit of memory, at the cost of having to fixup some of the values
- * in the section headers.
+ * There are two kinds of variations that can occur: the relative
+ * virtual address of the section might be different from the
+ * physical file offset, and the virtual section size might be
+ * different from the physical size (for example, the physical
+ * size of the .data section might be 1024 bytes, but the virtual
+ * size might be 1384 bytes, indicating that the data section should
+ * actually use up 1384 bytes in RAM and be padded with zeros). What we
+ * do is read the original file into memory and then make an in-memory
+ * copy with all of the sections relocated, re-sized and zero padded
+ * according to the virtual values specified in the section headers.
+ * We then emit the fixed up image file for use by the if_ndis driver.
+ * This way, we don't have to do the fixups inside the kernel.
  */
 
-#define ROUND_UP(x, y)	\
-	(((x) + (y)) - ((x) % (y)))
+#define ROUND_DOWN(n, align)    (((uintptr_t)n) & ~((align) - 1l))
+#define ROUND_UP(n, align)      ROUND_DOWN(((uintptr_t)n) + (align) - 1l, \
+                                (align))
 
 #define SET_HDRS(x)	\
 	dos_hdr = (image_dos_header *)x;				\
@@ -92,7 +101,7 @@ int insert_padding(imgbase, imglen)
         image_nt_header		*nt_hdr;
 	image_optional_header	opt_hdr;
         int			i = 0, sections, curlen = 0;
-	int			offaccum = 0, diff, oldraddr, oldrlen;
+	int			offaccum = 0, oldraddr, oldrlen;
 	uint8_t			*newimg, *tmp;
 
 	newimg = malloc(*imglen);
@@ -111,35 +120,24 @@ int insert_padding(imgbase, imglen)
 	SET_HDRS(newimg);
 
 	for (i = 0; i < sections; i++) {
-		/*
-		 * If we have accumulated any padding offset,
-		 * add it to the raw data address of this segment.
-		 */
 		oldraddr = sect_hdr->ish_rawdataaddr;
 		oldrlen = sect_hdr->ish_rawdatasize;
-		if (offaccum)
-			sect_hdr->ish_rawdataaddr += offaccum;
-		if (sect_hdr->ish_misc.ish_vsize >
-		    sect_hdr->ish_rawdatasize) {
-			diff = ROUND_UP(sect_hdr->ish_misc.ish_vsize -
-			    sect_hdr->ish_rawdatasize,
-			    opt_hdr.ioh_filealign);
-			offaccum += ROUND_UP(diff -
-			    (sect_hdr->ish_misc.ish_vsize -
-			    sect_hdr->ish_rawdatasize),
-			    opt_hdr.ioh_filealign);
-			sect_hdr->ish_rawdatasize =
-			    ROUND_UP(sect_hdr->ish_rawdatasize,
-			    opt_hdr.ioh_filealign);
-			tmp = realloc(newimg, *imglen + offaccum);
-			if (tmp == NULL) {
-				free(newimg);
-				return(ENOMEM);
-			}
-			newimg = tmp;
-			SET_HDRS(newimg);
-			sect_hdr += i;
+		sect_hdr->ish_rawdataaddr = sect_hdr->ish_vaddr;
+		offaccum += ROUND_UP(sect_hdr->ish_vaddr - oldraddr,
+		    opt_hdr.ioh_filealign);
+		offaccum +=
+		    ROUND_UP(sect_hdr->ish_misc.ish_vsize,
+			     opt_hdr.ioh_filealign) -
+		    ROUND_UP(sect_hdr->ish_rawdatasize,
+			     opt_hdr.ioh_filealign);
+		tmp = realloc(newimg, *imglen + offaccum);
+		if (tmp == NULL) {
+			free(newimg);
+			return(ENOMEM);
 		}
+		newimg = tmp;
+		SET_HDRS(newimg);
+		sect_hdr += i;
 		bzero(newimg + sect_hdr->ish_rawdataaddr,
 		    ROUND_UP(sect_hdr->ish_misc.ish_vsize,
 		    opt_hdr.ioh_filealign));