libipt: check section boundaries in pt_section_read()
When reading from a mapped section, we perform all kinds of overflow checks in
the various read functions. And we still missed an overflow on systems with
32-bit pointers.
Move the check to pt_section_read() where we check the requested offset against
the section size. Also truncate the read request there, if necessary.
This fixes the overflow on systems with 32-bit pointers.
Change-Id: I3c4591dcde113e2696347298af5ad3671ef0e41c
Signed-off-by: Markus Metzger <markus.t.metzger@intel.com>
diff --git a/libipt/src/posix/pt_section_posix.c b/libipt/src/posix/pt_section_posix.c
index 8da97e0..1b1ce05 100644
--- a/libipt/src/posix/pt_section_posix.c
+++ b/libipt/src/posix/pt_section_posix.c
@@ -246,8 +246,7 @@
uint16_t size, uint64_t offset)
{
struct pt_sec_posix_mapping *mapping;
- const uint8_t *begin, *end;
- int bytes;
+ const uint8_t *begin;
if (!buffer || !section)
return -pte_invalid;
@@ -256,23 +255,14 @@
if (!mapping)
return -pte_internal;
+ /* We already checked in pt_section_read() that the requested memory
+ * lies within the section's boundaries.
+ *
+ * And we checked that the entire section was mapped. There's no need
+ * to check for overflows, again.
+ */
begin = mapping->begin + offset;
- end = begin + size;
- if (end < begin)
- return -pte_nomap;
-
- if (mapping->end <= begin)
- return -pte_nomap;
-
- if (begin < mapping->begin)
- return -pte_nomap;
-
- if (mapping->end < end)
- end = mapping->end;
-
- bytes = (int) (end - begin);
-
- memcpy(buffer, begin, bytes);
- return bytes;
+ memcpy(buffer, begin, size);
+ return (int) size;
}
diff --git a/libipt/src/pt_section.c b/libipt/src/pt_section.c
index 87d242e..e04275a 100644
--- a/libipt/src/pt_section.c
+++ b/libipt/src/pt_section.c
@@ -265,11 +265,22 @@
int pt_section_read(const struct pt_section *section, uint8_t *buffer,
uint16_t size, uint64_t offset)
{
+ uint64_t limit, space;
+
if (!section)
return -pte_internal;
if (!section->read)
return -pte_nomap;
+ limit = section->size;
+ if (limit <= offset)
+ return -pte_nomap;
+
+ /* Truncate if we try to read past the end of the section. */
+ space = limit - offset;
+ if (space < size)
+ size = (uint16_t) space;
+
return section->read(section, buffer, size, offset);
}
diff --git a/libipt/src/pt_section_file.c b/libipt/src/pt_section_file.c
index 0ae13f8..034352c 100644
--- a/libipt/src/pt_section_file.c
+++ b/libipt/src/pt_section_file.c
@@ -195,7 +195,7 @@
{
struct pt_sec_file_mapping *mapping;
FILE *file;
- long begin, end, fbegin, fend;
+ long begin;
size_t read;
int errcode;
@@ -207,30 +207,20 @@
return -pte_internal;
file = mapping->file;
- begin = mapping->begin;
- end = mapping->end;
- fbegin = (long) offset;
- if (((uint64_t) fbegin) != offset)
- return -pte_nomap;
-
- fbegin += begin;
- fend = fbegin + size;
-
- if (fend < fbegin)
- return -pte_nomap;
-
- if (end <= fbegin)
- return -pte_nomap;
-
- if (fbegin < begin)
- return -pte_nomap;
+ /* We already checked in pt_section_read() that the requested memory
+ * lies within the section's boundaries.
+ *
+ * And we checked that the file covers the entire section in
+ * pt_sec_file_map(). There's no need to check for overflows, again.
+ */
+ begin = mapping->begin + (long) offset;
errcode = fmap_lock(mapping);
if (errcode < 0)
return errcode;
- errcode = fseek(file, fbegin, SEEK_SET);
+ errcode = fseek(file, begin, SEEK_SET);
if (errcode)
goto out_unlock;
diff --git a/libipt/src/windows/pt_section_windows.c b/libipt/src/windows/pt_section_windows.c
index 02dcb06..2fc6000 100644
--- a/libipt/src/windows/pt_section_windows.c
+++ b/libipt/src/windows/pt_section_windows.c
@@ -321,8 +321,7 @@
uint16_t size, uint64_t offset)
{
struct pt_sec_windows_mapping *mapping;
- const uint8_t *begin, *end;
- int bytes;
+ const uint8_t *begin;
if (!buffer || !section)
return -pte_invalid;
@@ -331,23 +330,14 @@
if (!mapping)
return -pte_internal;
+ /* We already checked in pt_section_read() that the requested memory
+ * lies within the section's boundaries.
+ *
+ * And we checked that the entire section was mapped. There's no need
+ * to check for overflows, again.
+ */
begin = mapping->begin + offset;
- end = begin + size;
- if (end < begin)
- return -pte_nomap;
-
- if (mapping->end <= begin)
- return -pte_nomap;
-
- if (begin < mapping->begin)
- return -pte_nomap;
-
- if (mapping->end < end)
- end = mapping->end;
-
- bytes = (int) (end - begin);
-
- memcpy(buffer, begin, bytes);
- return bytes;
+ memcpy(buffer, begin, size);
+ return (int) size;
}
diff --git a/libipt/test/src/ptunit-section.c b/libipt/test/src/ptunit-section.c
index cb9ce20..38f676f 100644
--- a/libipt/test/src/ptunit-section.c
+++ b/libipt/test/src/ptunit-section.c
@@ -491,6 +491,30 @@
return ptu_passed();
}
+static struct ptunit_result read_overflow_32bit(struct section_fixture *sfix)
+{
+ uint8_t bytes[] = { 0xcc, 0x2, 0x4, 0x6 }, buffer[] = { 0xcc };
+ int status;
+
+ sfix_write(sfix, bytes);
+
+ sfix->section = pt_mk_section(sfix->name, 0x1ull, 0x3ull);
+ ptu_ptr(sfix->section);
+
+ status = pt_section_map(sfix->section);
+ ptu_int_eq(status, 0);
+
+ status = pt_section_read(sfix->section, buffer, 1,
+ 0xff00000000ull);
+ ptu_int_eq(status, -pte_nomap);
+ ptu_uint_eq(buffer[0], 0xcc);
+
+ status = pt_section_unmap(sfix->section);
+ ptu_int_eq(status, 0);
+
+ return ptu_passed();
+}
+
static struct ptunit_result read_nomap(struct section_fixture *sfix)
{
uint8_t bytes[] = { 0xcc, 0x2, 0x4, 0x6 }, buffer[] = { 0xcc };
@@ -706,6 +730,7 @@
ptu_run_f(suite, read_from_truncated, sfix);
ptu_run_f(suite, read_nomem, sfix);
ptu_run_f(suite, read_overflow, sfix);
+ ptu_run_f(suite, read_overflow_32bit, sfix);
ptu_run_f(suite, read_nomap, sfix);
ptu_run_f(suite, read_unmap_map, sfix);
ptu_run_f(suite, stress, sfix);
