[PATCH] makedumpfile: Fix several issues with reading ELF pages

[Petr Tesarik]

While adopting the algorithm for libkdumpfile, several corner cases
were found by a test case:

  1. If the last part of a page is not present in the ELF file, it
     should be replaced with zeroes. However, the check is incorrect.

  2. If the beginning of a page is not present, following data is
     read from an incorrect file offset.

  3. If the page is split among several segments, the current
     position in the buffer is not always taken into account.

Case 1 is a simple typo/braino (writing bufptr instead of endp).

To fix cases 2 and 3, it is best to update the paddr variable so that
it always corresponds to the current read position in the buffer.

I have tested the new code with a specially crafted ELF dump where one
page had the following (artificial) layout:

  #  PAGE RANGE    STORED IN   DATA
  --------------------------------------------------
  1  0x000-0x007   nowhere     fake zero
  2  0x008-0x067   LOAD #1     read from file
  3  0x068-0x06f   nowhere     fake zero
  4  0x070-0x13f   LOAD #2     read from file
  5  0x140-0x147   LOAD #2     zero (memsz > filesz)
  6  0x148-0xff7   LOAD #3     read from file
  7  0xff8-0xfff   nowhere     fake zero

Case 1 tests the conditional right after get_pt_load_extents().
Case 2 tests file read after missing data.
Case 3 tests the conditional from case 1 with non-zero buffer position.
Case 5 tests the last conditional in the loop (p < endp).
Case 6 tests exact match in get_pt_load_extents().
Case 7 tests the final conditional after the loop.

Signed-off-by: Petr Tesarik <ptesarik at suse.com>

---
 makedumpfile.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -656,14 +656,15 @@ readpage_elf(unsigned long long paddr, v
 	p = bufptr;
 	endp = p + info->page_size;
 	while (p < endp) {
-		idx = closest_pt_load(paddr + (p - bufptr), endp - p);
+		idx = closest_pt_load(paddr, endp - p);
 		if (idx < 0)
 			break;
 
 		get_pt_load_extents(idx, &phys_start, &phys_end, &offset, &size);
-		if (phys_start > paddr + (p - bufptr)) {
+		if (phys_start > paddr) {
 			memset(p, 0, phys_start - paddr);
 			p += phys_start - paddr;
+			paddr = phys_start;
 		}
 
 		offset += paddr - phys_start;
@@ -677,6 +678,7 @@ readpage_elf(unsigned long long paddr, v
 				return FALSE;
 			}
 			p += size;
+			paddr += size;
 		}
 		if (p < endp) {
 			size = phys_end - paddr;
@@ -684,6 +686,7 @@ readpage_elf(unsigned long long paddr, v
 				size = endp - p;
 			memset(p, 0, size);
 			p += size;
+			paddr += size;
 		}
 	}
 
@@ -691,7 +694,7 @@ readpage_elf(unsigned long long paddr, v
 		ERRMSG("Attempt to read non-existent page at 0x%llx.\n",
 		       paddr);
 		return FALSE;
-	} else if (p < bufptr)
+	} else if (p < endp)
 		memset(p, 0, endp - p);
 
 	return TRUE;
@@ -708,14 +711,15 @@ readpage_elf_parallel(int fd_memory, uns
 	p = bufptr;
 	endp = p + info->page_size;
 	while (p < endp) {
-		idx = closest_pt_load(paddr + (p - bufptr), endp - p);
+		idx = closest_pt_load(paddr, endp - p);
 		if (idx < 0)
 			break;
 
 		get_pt_load_extents(idx, &phys_start, &phys_end, &offset, &size);
-		if (phys_start > paddr + (p - bufptr)) {
+		if (phys_start > paddr) {
 			memset(p, 0, phys_start - paddr);
 			p += phys_start - paddr;
+			paddr = phys_start;
 		}
 
 		offset += paddr - phys_start;
@@ -730,6 +734,7 @@ readpage_elf_parallel(int fd_memory, uns
 				return FALSE;
 			}
 			p += size;
+			paddr += size;
 		}
 		if (p < endp) {
 			size = phys_end - paddr;
@@ -737,6 +742,7 @@ readpage_elf_parallel(int fd_memory, uns
 				size = endp - p;
 			memset(p, 0, size);
 			p += size;
+			paddr += size;
 		}
 	}
 
@@ -744,7 +750,7 @@ readpage_elf_parallel(int fd_memory, uns
 		ERRMSG("Attempt to read non-existent page at 0x%llx.\n",
 		       paddr);
 		return FALSE;
-	} else if (p < bufptr)
+	} else if (p < endp)
 		memset(p, 0, endp - p);
 
 	return TRUE;