linux-kernel - [PATCH] staging: gs_fpgaboot: add buffer overflow checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170717003841.6317-1-jacobvonchorus@cwphoto.ca>
Date:   Sun, 16 Jul 2017 20:38:41 -0400
From:   Jacob von Chorus <jacobvonchorus@...hoto.ca>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Insop Song <insop.song@...nspeed.com>
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        Jacob von Chorus <jacobvonchorus@...hoto.ca>
Subject: [PATCH] staging: gs_fpgaboot: add buffer overflow checks

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.

This patch also fixes a checkpatch.pl CHECK in io.c by removing the FSF
address paragraph.

Signed-off-by: Jacob von Chorus <jacobvonchorus@...hoto.ca>
---
 drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 39 ++++++++++++++++++++++---------
 drivers/staging/gs_fpgaboot/io.c          |  4 ----
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..2aafd769b8 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 n, 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 > n) {
+		pr_err("error: readinfo buffer too small\n");
+		return -1;
+	}
 
 	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,7 +133,7 @@ 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;
@@ -135,26 +141,37 @@ static void gs_read_bitstream(struct fpgaimage *fimage)
 	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);
+	if (readmagic_bitstream(bitdata, &offset))
+		return -1;
+
+	if (readinfo_bitstream(bitdata, fimage->filename, MAX_STR, &offset))
+		return -1;
+	if (readinfo_bitstream(bitdata, fimage->part, MAX_STR, &offset))
+		return -1;
+	if (readinfo_bitstream(bitdata, fimage->date, MAX_STR, &offset))
+		return -1;
+	if (readinfo_bitstream(bitdata, fimage->time, MAX_STR, &offset))
+		return -1;
+
+	if (readlength_bitstream(bitdata, &fimage->lendata, &offset))
+		return -1;
 
 	fimage->fpgadata = bitdata + offset;
+
+	return 0;
 }
 
 static int gs_read_image(struct fpgaimage *fimage)
 {
 	int img_fmt;
 
-	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);
+		if (gs_read_bitstream(fimage))
+			return -1;
 		break;
 	default:
 		pr_err("unsupported fpga image format\n");
diff --git a/drivers/staging/gs_fpgaboot/io.c b/drivers/staging/gs_fpgaboot/io.c
index c9391198fb..83a13ca725 100644
--- a/drivers/staging/gs_fpgaboot/io.c
+++ b/drivers/staging/gs_fpgaboot/io.c
@@ -9,10 +9,6 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
 #include <linux/kernel.h>
-- 
2.11.0