[PATCH v2 1/2] staging: gs_fpgaboot: add buffer overflow checks

* [PATCH v2 1/2] staging: gs_fpgaboot: add buffer overflow checks
@ 2017-07-18  0:47 Jacob von Chorus
  2017-07-18  0:47 ` [PATCH v2 2/2] staging: gs_fpgaboot: change char to u8 Jacob von Chorus
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jacob von Chorus @ 2017-07-18  0:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Insop Song
  Cc: devel, linux-kernel, dan.carpenter, Jacob von Chorus

Four fields in struct fpgaimage are char arrays of length MAX_STR (256).
The amount of data read into these buffers is controlled by a length
field in the bitstream file read from userspace. If a corrupt or
malicious firmware file was supplied, kernel data beyond these buffers
can be overwritten arbitrarily.

This patch adds a check of the bitstream's length value to ensure it
fits within the bounds of the allocated buffers. An error condition is
returned from gs_read_bitstream if any of the reads fail.

Signed-off-by: Jacob von Chorus <jacobvonchorus@cwphoto.ca>
---
 drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 48 ++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..008ef99f05 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -47,7 +47,7 @@ static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
 	*offset += rdsize;
 }
 
-static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
+static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
 {
 	char tbuf[64];
 	s32 len;
@@ -59,9 +59,15 @@ static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
 	read_bitstream(bitdata, tbuf, offset, 2);
 
 	len = tbuf[0] << 8 | tbuf[1];
+	if ((len + 1) > size) {
+		pr_err("error: readinfo buffer too small\n");
+		return -ETOOSMALL;
+	}
 
 	read_bitstream(bitdata, buf, offset, len);
 	buf[len] = '\0';
+
+	return 0;
 }
 
 /*
@@ -113,7 +119,7 @@ static int readmagic_bitstream(char *bitdata, int *offset)
 /*
  * NOTE: supports only bitstream format
  */
-static enum fmt_image get_imageformat(struct fpgaimage *fimage)
+static enum fmt_image get_imageformat(void)
 {
 	return f_bit;
 }
@@ -127,34 +133,54 @@ static void gs_print_header(struct fpgaimage *fimage)
 	pr_info("lendata: %d\n", fimage->lendata);
 }
 
-static void gs_read_bitstream(struct fpgaimage *fimage)
+static int gs_read_bitstream(struct fpgaimage *fimage)
 {
 	char *bitdata;
 	int offset;
+	int err;
 
 	offset = 0;
 	bitdata = (char *)fimage->fw_entry->data;
 
-	readmagic_bitstream(bitdata, &offset);
-	readinfo_bitstream(bitdata, fimage->filename, &offset);
-	readinfo_bitstream(bitdata, fimage->part, &offset);
-	readinfo_bitstream(bitdata, fimage->date, &offset);
-	readinfo_bitstream(bitdata, fimage->time, &offset);
-	readlength_bitstream(bitdata, &fimage->lendata, &offset);
+	err = readmagic_bitstream(bitdata, &offset);
+	if (err)
+		return err;
+
+	err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, &offset);
+	if (err)
+		return err;
+	err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, &offset);
+	if (err)
+		return err;
+	err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, &offset);
+	if (err)
+		return err;
+	err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, &offset);
+	if (err)
+		return err;
+
+	err = readlength_bitstream(bitdata, &fimage->lendata, &offset);
+	if (err)
+		return err;
 
 	fimage->fpgadata = bitdata + offset;
+
+	return 0;
 }
 
 static int gs_read_image(struct fpgaimage *fimage)
 {
 	int img_fmt;
+	int err;
 
-	img_fmt = get_imageformat(fimage);
+	img_fmt = get_imageformat();
 
 	switch (img_fmt) {
 	case f_bit:
 		pr_info("image is bitstream format\n");
-		gs_read_bitstream(fimage);
+		err = gs_read_bitstream(fimage);
+		if (err)
+			return err;
 		break;
 	default:
 		pr_err("unsupported fpga image format\n");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread