[PATCH v3/resubmit 1/3] 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.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
v3:
- use >= to prevent an integer overflow in the comparison
- use get_unaligned_be functions to interpret length fields
- fix remainder of file to use valid error codes
v2:
- char arrays converted to u8 arrays
- replace error return value with proper error code in
gs_read_bitstream
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..a49019af60 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "gs_fpgaboot.h"
#include "io.h"
@@ -47,7 +48,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;
@@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf,
int *offset)
/* read length */
read_bitstream(bitdata, tbuf, offset, 2);
- len = tbuf[0] << 8 | tbuf[1];
+ len = get_unaligned_be16(tbuf);
+ if (len >= size) {
+ pr_err("error: readinfo buffer too small\n");
+ return -EINVAL;
+ }
read_bitstream(bitdata, buf, offset, len);
buf[len] = '\0';
+
+ return 0;
}
/*
@@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/* read 4bytes length */
read_bitstream(bitdata, tbuf, offset, 4);
- *lendata = tbuf[0] << 24 | tbuf[1] << 16 |
- tbuf[2] << 8 | tbuf[3];
+ *lendata = get_unaligned_be32(tbuf);
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
[PATCH v3/resubmit 1/3] 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.
Signed-off-by: Jacob von Chorus
---
v3:
- use >= to prevent an integer overflow in the comparison
- use get_unaligned_be functions to interpret length fields
- fix remainder of file to use valid error codes
v2:
- char arrays converted to u8 arrays
- replace error return value with proper error code in
gs_read_bitstream
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..a49019af60 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "gs_fpgaboot.h"
#include "io.h"
@@ -47,7 +48,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;
@@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf,
int *offset)
/* read length */
read_bitstream(bitdata, tbuf, offset, 2);
- len = tbuf[0] << 8 | tbuf[1];
+ len = get_unaligned_be16(tbuf);
+ if (len >= size) {
+ pr_err("error: readinfo buffer too small\n");
+ return -EINVAL;
+ }
read_bitstream(bitdata, buf, offset, len);
buf[len] = '\0';
+
+ return 0;
}
/*
@@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/* read 4bytes length */
read_bitstream(bitdata, tbuf, offset, 4);
- *lendata = tbuf[0] << 24 | tbuf[1] << 16 |
- tbuf[2] << 8 | tbuf[3];
+ *lendata = get_unaligned_be32(tbuf);
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
[PATCH v3/resubmit 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
v3:
- reduce temporary buffer size in bitstream reading functions
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index a49019af60..ff59708792 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[2];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[4];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v3/resubmit 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus
---
v3:
- reduce temporary buffer size in bitstream reading functions
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index a49019af60..ff59708792 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[2];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[4];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v3/resubmit 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses
error codes are propagated back to the init function's return.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index ff59708792..bcbdc7340b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata,
int *offset)
/* make sure it is section 'e' */
if (tbuf[0] != 'e') {
pr_err("error: length section is not 'e', but %c\n", tbuf[0]);
- return -1;
+ return -EINVAL;
}
/* read 4bytes length */
@@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset)
r = memcmp(buf, bits_magic, 13);
if (r) {
pr_err("error: corrupted header");
- return -1;
+ return -EINVAL;
}
pr_info("bitstream file magic number Ok\n");
@@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage)
break;
default:
pr_err("unsupported fpga image format\n");
- return -1;
+ return -EINVAL;
}
gs_print_header(fimage);
@@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (!xl_supported_prog_bus_width(bus_bytes)) {
pr_err("unsupported program bus width %d\n",
bus_bytes);
- return -1;
+ return -EINVAL;
}
/* Bring csi_b, rdwr_b Low and program_b High */
@@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
/* Check INIT_B */
if (xl_get_init_b() == 0) {
pr_err("init_b 0\n");
- return -1;
+ return -EIO;
}
while (xl_get_done_b() == 0) {
@@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (cnt > MAX_WAIT_DONE) {
pr_err("fpga download fail\n");
- return -1;
+ return -EIO;
}
pr_info("download fpgaimage\n");
@@ -351,7 +351,7 @@ static int gs_fpgaboot(void)
err_out1:
kfree(fimage);
- return -1;
+ return err;
}
static int __init gs_fpgaboot_init(void)
--
2.11.0
[PATCH v3/resubmit 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses
error codes are propagated back to the init function's return.
Signed-off-by: Jacob von Chorus
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index ff59708792..bcbdc7340b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata,
int *offset)
/* make sure it is section 'e' */
if (tbuf[0] != 'e') {
pr_err("error: length section is not 'e', but %c\n", tbuf[0]);
- return -1;
+ return -EINVAL;
}
/* read 4bytes length */
@@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset)
r = memcmp(buf, bits_magic, 13);
if (r) {
pr_err("error: corrupted header");
- return -1;
+ return -EINVAL;
}
pr_info("bitstream file magic number Ok\n");
@@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage)
break;
default:
pr_err("unsupported fpga image format\n");
- return -1;
+ return -EINVAL;
}
gs_print_header(fimage);
@@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (!xl_supported_prog_bus_width(bus_bytes)) {
pr_err("unsupported program bus width %d\n",
bus_bytes);
- return -1;
+ return -EINVAL;
}
/* Bring csi_b, rdwr_b Low and program_b High */
@@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
/* Check INIT_B */
if (xl_get_init_b() == 0) {
pr_err("init_b 0\n");
- return -1;
+ return -EIO;
}
while (xl_get_done_b() == 0) {
@@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (cnt > MAX_WAIT_DONE) {
pr_err("fpga download fail\n");
- return -1;
+ return -EIO;
}
pr_info("download fpgaimage\n");
@@ -351,7 +351,7 @@ static int gs_fpgaboot(void)
err_out1:
kfree(fimage);
- return -1;
+ return err;
}
static int __init gs_fpgaboot_init(void)
--
2.11.0
[PATCH v3/resubmit 1/3] 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.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
v3:
- use >= to prevent an integer overflow in the comparison
- use get_unaligned_be functions to interpret length fields
- fix remainder of file to use valid error codes
v2:
- char arrays converted to u8 arrays
- replace error return value with proper error code in
gs_read_bitstream
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..a49019af60 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "gs_fpgaboot.h"
#include "io.h"
@@ -47,7 +48,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;
@@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf,
int *offset)
/* read length */
read_bitstream(bitdata, tbuf, offset, 2);
- len = tbuf[0] << 8 | tbuf[1];
+ len = get_unaligned_be16(tbuf);
+ if (len >= size) {
+ pr_err("error: readinfo buffer too small\n");
+ return -EINVAL;
+ }
read_bitstream(bitdata, buf, offset, len);
buf[len] = '\0';
+
+ return 0;
}
/*
@@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/* read 4bytes length */
read_bitstream(bitdata, tbuf, offset, 4);
- *lendata = tbuf[0] << 24 | tbuf[1] << 16 |
- tbuf[2] << 8 | tbuf[3];
+ *lendata = get_unaligned_be32(tbuf);
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
[PATCH v3/resubmit 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
v3:
- reduce temporary buffer size in bitstream reading functions
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index a49019af60..ff59708792 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[2];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[4];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v3/resubmit 1/3] 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.
Signed-off-by: Jacob von Chorus
v3:
- use >= to prevent an integer overflow in the comparison
- use get_unaligned_be functions to interpret length fields
- fix remainder of file to use valid error codes
v2:
- char arrays converted to u8 arrays
- replace error return value with proper error code in
gs_read_bitstream
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..a49019af60 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "gs_fpgaboot.h"
#include "io.h"
@@ -47,7 +48,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;
@@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf,
int *offset)
/* read length */
read_bitstream(bitdata, tbuf, offset, 2);
- len = tbuf[0] << 8 | tbuf[1];
+ len = get_unaligned_be16(tbuf);
+ if (len >= size) {
+ pr_err("error: readinfo buffer too small\n");
+ return -EINVAL;
+ }
read_bitstream(bitdata, buf, offset, len);
buf[len] = '\0';
+
+ return 0;
}
/*
@@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/* read 4bytes length */
read_bitstream(bitdata, tbuf, offset, 4);
- *lendata = tbuf[0] << 24 | tbuf[1] << 16 |
- tbuf[2] << 8 | tbuf[3];
+ *lendata = get_unaligned_be32(tbuf);
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
[PATCH v3/resubmit 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus
v3:
- reduce temporary buffer size in bitstream reading functions
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index a49019af60..ff59708792 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[2];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[4];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v3/resubmit 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses
error codes are propagated back to the init function's return.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index ff59708792..bcbdc7340b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata,
int *offset)
/* make sure it is section 'e' */
if (tbuf[0] != 'e') {
pr_err("error: length section is not 'e', but %c\n", tbuf[0]);
- return -1;
+ return -EINVAL;
}
/* read 4bytes length */
@@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset)
r = memcmp(buf, bits_magic, 13);
if (r) {
pr_err("error: corrupted header");
- return -1;
+ return -EINVAL;
}
pr_info("bitstream file magic number Ok\n");
@@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage)
break;
default:
pr_err("unsupported fpga image format\n");
- return -1;
+ return -EINVAL;
}
gs_print_header(fimage);
@@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (!xl_supported_prog_bus_width(bus_bytes)) {
pr_err("unsupported program bus width %d\n",
bus_bytes);
- return -1;
+ return -EINVAL;
}
/* Bring csi_b, rdwr_b Low and program_b High */
@@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
/* Check INIT_B */
if (xl_get_init_b() == 0) {
pr_err("init_b 0\n");
- return -1;
+ return -EIO;
}
while (xl_get_done_b() == 0) {
@@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (cnt > MAX_WAIT_DONE) {
pr_err("fpga download fail\n");
- return -1;
+ return -EIO;
}
pr_info("download fpgaimage\n");
@@ -351,7 +351,7 @@ static int gs_fpgaboot(void)
err_out1:
kfree(fimage);
- return -1;
+ return err;
}
static int __init gs_fpgaboot_init(void)
--
2.11.0
[PATCH v3/resubmit 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses
error codes are propagated back to the init function's return.
Signed-off-by: Jacob von Chorus
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index ff59708792..bcbdc7340b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata,
int *offset)
/* make sure it is section 'e' */
if (tbuf[0] != 'e') {
pr_err("error: length section is not 'e', but %c\n", tbuf[0]);
- return -1;
+ return -EINVAL;
}
/* read 4bytes length */
@@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset)
r = memcmp(buf, bits_magic, 13);
if (r) {
pr_err("error: corrupted header");
- return -1;
+ return -EINVAL;
}
pr_info("bitstream file magic number Ok\n");
@@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage)
break;
default:
pr_err("unsupported fpga image format\n");
- return -1;
+ return -EINVAL;
}
gs_print_header(fimage);
@@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (!xl_supported_prog_bus_width(bus_bytes)) {
pr_err("unsupported program bus width %d\n",
bus_bytes);
- return -1;
+ return -EINVAL;
}
/* Bring csi_b, rdwr_b Low and program_b High */
@@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
/* Check INIT_B */
if (xl_get_init_b() == 0) {
pr_err("init_b 0\n");
- return -1;
+ return -EIO;
}
while (xl_get_done_b() == 0) {
@@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (cnt > MAX_WAIT_DONE) {
pr_err("fpga download fail\n");
- return -1;
+ return -EIO;
}
pr_info("download fpgaimage\n");
@@ -351,7 +351,7 @@ static int gs_fpgaboot(void)
err_out1:
kfree(fimage);
- return -1;
+ return err;
}
static int __init gs_fpgaboot_init(void)
--
2.11.0
[PATCH v3 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
v3:
- reduce temporary buffer size in bitstream reading functions
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index a49019af60..ff59708792 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[2];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[4];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v3 2/3] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus
v3:
- reduce temporary buffer size in bitstream reading functions
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index a49019af60..ff59708792 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -42,16 +42,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[2];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -74,9 +74,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[4];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v3 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses
error codes are propagated back to the init function's return.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index ff59708792..bcbdc7340b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata,
int *offset)
/* make sure it is section 'e' */
if (tbuf[0] != 'e') {
pr_err("error: length section is not 'e', but %c\n", tbuf[0]);
- return -1;
+ return -EINVAL;
}
/* read 4bytes length */
@@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset)
r = memcmp(buf, bits_magic, 13);
if (r) {
pr_err("error: corrupted header");
- return -1;
+ return -EINVAL;
}
pr_info("bitstream file magic number Ok\n");
@@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage)
break;
default:
pr_err("unsupported fpga image format\n");
- return -1;
+ return -EINVAL;
}
gs_print_header(fimage);
@@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (!xl_supported_prog_bus_width(bus_bytes)) {
pr_err("unsupported program bus width %d\n",
bus_bytes);
- return -1;
+ return -EINVAL;
}
/* Bring csi_b, rdwr_b Low and program_b High */
@@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
/* Check INIT_B */
if (xl_get_init_b() == 0) {
pr_err("init_b 0\n");
- return -1;
+ return -EIO;
}
while (xl_get_done_b() == 0) {
@@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (cnt > MAX_WAIT_DONE) {
pr_err("fpga download fail\n");
- return -1;
+ return -EIO;
}
pr_info("download fpgaimage\n");
@@ -351,7 +351,7 @@ static int gs_fpgaboot(void)
err_out1:
kfree(fimage);
- return -1;
+ return err;
}
static int __init gs_fpgaboot_init(void)
--
2.11.0
[PATCH v3 3/3] staging: gs_fpgaboot: return valid error codes
The return values on error are modified to be valid error codes. Theses
error codes are propagated back to the init function's return.
Signed-off-by: Jacob von Chorus
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index ff59708792..bcbdc7340b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -84,7 +84,7 @@ static int readlength_bitstream(u8 *bitdata, int *lendata,
int *offset)
/* make sure it is section 'e' */
if (tbuf[0] != 'e') {
pr_err("error: length section is not 'e', but %c\n", tbuf[0]);
- return -1;
+ return -EINVAL;
}
/* read 4bytes length */
@@ -107,7 +107,7 @@ static int readmagic_bitstream(u8 *bitdata, int *offset)
r = memcmp(buf, bits_magic, 13);
if (r) {
pr_err("error: corrupted header");
- return -1;
+ return -EINVAL;
}
pr_info("bitstream file magic number Ok\n");
@@ -184,7 +184,7 @@ static int gs_read_image(struct fpgaimage *fimage)
break;
default:
pr_err("unsupported fpga image format\n");
- return -1;
+ return -EINVAL;
}
gs_print_header(fimage);
@@ -223,7 +223,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (!xl_supported_prog_bus_width(bus_bytes)) {
pr_err("unsupported program bus width %d\n",
bus_bytes);
- return -1;
+ return -EINVAL;
}
/* Bring csi_b, rdwr_b Low and program_b High */
@@ -250,7 +250,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
/* Check INIT_B */
if (xl_get_init_b() == 0) {
pr_err("init_b 0\n");
- return -1;
+ return -EIO;
}
while (xl_get_done_b() == 0) {
@@ -262,7 +262,7 @@ static int gs_download_image(struct fpgaimage *fimage, enum
wbus bus_bytes)
if (cnt > MAX_WAIT_DONE) {
pr_err("fpga download fail\n");
- return -1;
+ return -EIO;
}
pr_info("download fpgaimage\n");
@@ -351,7 +351,7 @@ static int gs_fpgaboot(void)
err_out1:
kfree(fimage);
- return -1;
+ return err;
}
static int __init gs_fpgaboot_init(void)
--
2.11.0
[PATCH v3 1/3] 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.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
v3:
- use >= to prevent an integer overflow in the comparison
- use get_unaligned_be functions to interpret length fields
- fix remainder of file to use valid error codes
v2:
- char arrays converted to u8 arrays
- replace error return value with proper error code in
gs_read_bitstream
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..a49019af60 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "gs_fpgaboot.h"
#include "io.h"
@@ -47,7 +48,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;
@@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf,
int *offset)
/* read length */
read_bitstream(bitdata, tbuf, offset, 2);
- len = tbuf[0] << 8 | tbuf[1];
+ len = get_unaligned_be16(tbuf);
+ if (len >= size) {
+ pr_err("error: readinfo buffer too small\n");
+ return -EINVAL;
+ }
read_bitstream(bitdata, buf, offset, len);
buf[len] = '\0';
+
+ return 0;
}
/*
@@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/* read 4bytes length */
read_bitstream(bitdata, tbuf, offset, 4);
- *lendata = tbuf[0] << 24 | tbuf[1] << 16 |
- tbuf[2] << 8 | tbuf[3];
+ *lendata = get_unaligned_be32(tbuf);
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
[PATCH v3 1/3] 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.
Signed-off-by: Jacob von Chorus
v3:
- use >= to prevent an integer overflow in the comparison
- use get_unaligned_be functions to interpret length fields
- fix remainder of file to use valid error codes
v2:
- char arrays converted to u8 arrays
- replace error return value with proper error code in
gs_read_bitstream
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 54 +++
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 19b550fff0..a49019af60 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "gs_fpgaboot.h"
#include "io.h"
@@ -47,7 +48,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;
@@ -58,10 +59,16 @@ static void readinfo_bitstream(char *bitdata, char *buf,
int *offset)
/* read length */
read_bitstream(bitdata, tbuf, offset, 2);
- len = tbuf[0] << 8 | tbuf[1];
+ len = get_unaligned_be16(tbuf);
+ if (len >= size) {
+ pr_err("error: readinfo buffer too small\n");
+ return -EINVAL;
+ }
read_bitstream(bitdata, buf, offset, len);
buf[len] = '\0';
+
+ return 0;
}
/*
@@ -83,8 +90,7 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/* read 4bytes length */
read_bitstream(bitdata, tbuf, offset, 4);
- *lendata = tbuf[0] << 24 | tbuf[1] << 16 |
- tbuf[2] << 8 | tbuf[3];
+ *lendata = get_unaligned_be32(tbuf);
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
Re: [PATCH v2 2/2] staging: gs_fpgaboot: change char to u8
On Mon, Jul 17, 2017 at 06:22:08PM -0700, Joe Perches wrote:
> read_bitstream takes an int rdsize, not a u16.
> and this function will overflow tbuf if len > 64
>
> static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
> {
> char tbuf[64];
> s32 len;
>
> /* read section char */
> read_bitstream(bitdata, tbuf, offset, 1);
>
> /* read length */
> read_bitstream(bitdata, tbuf, offset, 2);
>
> len = tbuf[0] << 8 | tbuf[1];
>
> read_bitstream(bitdata, buf, offset, len);
> buf[len] = '\0';
> }
>
> len is up to 64k but tbuf is 64 bytes.
tbuf is used here to read a total of 3 bytes over two calls to
read_bitstream. The larger read of size, len, is stored to buf which is
MAX_STR bytes in length.
> len = get_unaligned_le16(tbuf)
>
> might be nicer than
>
> len = tbuf[0] << 8 | tbuf[1];
Agreed, though it should be "get_unaligned_be16".
Thanks.
Regards,
Jacob von Chorus
Re: [PATCH v2 2/2] staging: gs_fpgaboot: change char to u8
On Mon, Jul 17, 2017 at 06:22:08PM -0700, Joe Perches wrote:
> read_bitstream takes an int rdsize, not a u16.
> and this function will overflow tbuf if len > 64
>
> static void readinfo_bitstream(char *bitdata, char *buf, int *offset)
> {
> char tbuf[64];
> s32 len;
>
> /* read section char */
> read_bitstream(bitdata, tbuf, offset, 1);
>
> /* read length */
> read_bitstream(bitdata, tbuf, offset, 2);
>
> len = tbuf[0] << 8 | tbuf[1];
>
> read_bitstream(bitdata, buf, offset, len);
> buf[len] = '\0';
> }
>
> len is up to 64k but tbuf is 64 bytes.
tbuf is used here to read a total of 3 bytes over two calls to
read_bitstream. The larger read of size, len, is stored to buf which is
MAX_STR bytes in length.
> len = get_unaligned_le16(tbuf)
>
> might be nicer than
>
> len = tbuf[0] << 8 | tbuf[1];
Agreed, though it should be "get_unaligned_be16".
Thanks.
Regards,
Jacob von Chorus
[PATCH v2 2/2] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 008ef99f05..467a0ad81f 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -41,16 +41,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[64];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -73,9 +73,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[64];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v2 2/2] staging: gs_fpgaboot: change char to u8
The bitstream storage variables were changed from char to u8 arrays to
prevent issues such as negative lengths. This change makes the code
compatible with the "data" field in "struct firmware" which is of type
u8.
Signed-off-by: Jacob von Chorus
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 24
drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 008ef99f05..467a0ad81f 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -41,16 +41,16 @@ static char *file = "xlinx_fpga_firmware.bit";
module_param(file, charp, 0444);
MODULE_PARM_DESC(file, "Xilinx FPGA firmware file.");
-static void read_bitstream(char *bitdata, char *buf, int *offset, int rdsize)
+static void read_bitstream(u8 *bitdata, u8 *buf, int *offset, int rdsize)
{
memcpy(buf, bitdata + *offset, rdsize);
*offset += rdsize;
}
-static int readinfo_bitstream(char *bitdata, char *buf, int size, int *offset)
+static int readinfo_bitstream(u8 *bitdata, u8 *buf, int size, int *offset)
{
- char tbuf[64];
- s32 len;
+ u8 tbuf[64];
+ u16 len;
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -73,9 +73,9 @@ static int readinfo_bitstream(char *bitdata, char *buf, int
size, int *offset)
/*
* read bitdata length
*/
-static int readlength_bitstream(char *bitdata, int *lendata, int *offset)
+static int readlength_bitstream(u8 *bitdata, int *lendata, int *offset)
{
- char tbuf[64];
+ u8 tbuf[64];
/* read section char */
read_bitstream(bitdata, tbuf, offset, 1);
@@ -98,9 +98,9 @@ static int readlength_bitstream(char *bitdata, int *lendata,
int *offset)
/*
* read first 13 bytes to check bitstream magic number
*/
-static int readmagic_bitstream(char *bitdata, int *offset)
+static int readmagic_bitstream(u8 *bitdata, int *offset)
{
- char buf[13];
+ u8 buf[13];
int r;
read_bitstream(bitdata, buf, offset, 13);
@@ -135,12 +135,12 @@ static void gs_print_header(struct fpgaimage *fimage)
static int gs_read_bitstream(struct fpgaimage *fimage)
{
- char *bitdata;
+ u8 *bitdata;
int offset;
int err;
offset = 0;
- bitdata = (char *)fimage->fw_entry->data;
+ bitdata = (u8 *)fimage->fw_entry->data;
err = readmagic_bitstream(bitdata, );
if (err)
@@ -209,11 +209,11 @@ static int gs_load_image(struct fpgaimage *fimage, char
*fw_file)
static int gs_download_image(struct fpgaimage *fimage, enum wbus bus_bytes)
{
- char *bitdata;
+ u8 *bitdata;
int size, i, cnt;
cnt = 0;
- bitdata = (char *)fimage->fpgadata;
+ bitdata = (u8 *)fimage->fpgadata;
size = fimage->lendata;
#ifdef DEBUG_FPGA
diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
index cd1eb2c4c9..986e841f6b 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h
@@ -47,5 +47,5 @@ struct fpgaimage {
chardate[MAX_STR];
chartime[MAX_STR];
int lendata;
- char*fpgadata;
+ u8 *fpgadata;
};
--
2.11.0
[PATCH v2 1/2] 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.
Signed-off-by: Jacob von Chorus <jacobvoncho...@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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
[PATCH v2 1/2] 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.
Signed-off-by: Jacob von Chorus
---
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ err = readmagic_bitstream(bitdata, );
+ if (err)
+ return err;
+
+ err = readinfo_bitstream(bitdata, fimage->filename, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->part, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->date, MAX_STR, );
+ if (err)
+ return err;
+ err = readinfo_bitstream(bitdata, fimage->time, MAX_STR, );
+ if (err)
+ return err;
+
+ err = readlength_bitstream(bitdata, >lendata, );
+ 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
Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks
On Mon, Jul 17, 2017 at 10:53:25PM +0300, Dan Carpenter wrote:
> > + if (len + 1 > n) {
>
> It's more idiomatic to say "if (len >= n)". Plus that's a good habbit
My reasoning behind using "((len + 1) > n)" is that len represents the length of
the string without null-termination. "buf" is required to store a
null-terminator on top of len. Using "len + 1" shows this requirement
more clearly; I will add brackets around "len + 1" for emphasis.
Thanks for the feedback, I will send a v2.
Regards,
Jacob von Chorus
Re: [PATCH] staging: gs_fpgaboot: add buffer overflow checks
On Mon, Jul 17, 2017 at 10:53:25PM +0300, Dan Carpenter wrote:
> > + if (len + 1 > n) {
>
> It's more idiomatic to say "if (len >= n)". Plus that's a good habbit
My reasoning behind using "((len + 1) > n)" is that len represents the length of
the string without null-termination. "buf" is required to store a
null-terminator on top of len. Using "len + 1" shows this requirement
more clearly; I will add brackets around "len + 1" for emphasis.
Thanks for the feedback, I will send a v2.
Regards,
Jacob von Chorus
[PATCH] staging: gs_fpgaboot: remove FSF address from GPL notice
This patch removes the FSF address from the GPL notice to fix a checkpatch.cl CHECK message. Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca> --- drivers/staging/gs_fpgaboot/io.c | 4 1 file changed, 4 deletions(-) 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 -- 2.11.0
[PATCH] staging: gs_fpgaboot: remove FSF address from GPL notice
This patch removes the FSF address from the GPL notice to fix a checkpatch.cl CHECK message. Signed-off-by: Jacob von Chorus --- drivers/staging/gs_fpgaboot/io.c | 4 1 file changed, 4 deletions(-) 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 -- 2.11.0
[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.
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 39 ++-
1 file changed, 28 insertions(+), 11 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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ if (readmagic_bitstream(bitdata, ))
+ return -1;
+
+ if (readinfo_bitstream(bitdata, fimage->filename, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->part, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->date, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->time, MAX_STR, ))
+ return -1;
+
+ if (readlength_bitstream(bitdata, >lendata, ))
+ 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");
--
2.11.0
[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.
Signed-off-by: Jacob von Chorus
---
drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 39 ++-
1 file changed, 28 insertions(+), 11 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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ if (readmagic_bitstream(bitdata, ))
+ return -1;
+
+ if (readinfo_bitstream(bitdata, fimage->filename, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->part, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->date, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->time, MAX_STR, ))
+ return -1;
+
+ if (readlength_bitstream(bitdata, >lendata, ))
+ 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");
--
2.11.0
[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 <jacobvoncho...@cwphoto.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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ if (readmagic_bitstream(bitdata, ))
+ return -1;
+
+ if (readinfo_bitstream(bitdata, fimage->filename, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->part, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->date, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->time, MAX_STR, ))
+ return -1;
+
+ if (readlength_bitstream(bitdata, >lendata, ))
+ 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
--
2.11.0
[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
---
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, );
- readinfo_bitstream(bitdata, fimage->filename, );
- readinfo_bitstream(bitdata, fimage->part, );
- readinfo_bitstream(bitdata, fimage->date, );
- readinfo_bitstream(bitdata, fimage->time, );
- readlength_bitstream(bitdata, >lendata, );
+ if (readmagic_bitstream(bitdata, ))
+ return -1;
+
+ if (readinfo_bitstream(bitdata, fimage->filename, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->part, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->date, MAX_STR, ))
+ return -1;
+ if (readinfo_bitstream(bitdata, fimage->time, MAX_STR, ))
+ return -1;
+
+ if (readlength_bitstream(bitdata, >lendata, ))
+ 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
--
2.11.0
[PATCH] thermal: core: move tz->device.groups cleanup to thermal_release
The device_unregister call in thermal_zone_device_unregister causes the
thermal_zone_device structure to be freed before the call to free the
dynamically allocated attribute groups. This leads to a kernel panic.
Furthermore, the 4 calls to free the trip point attribute structures
occur before the call to unregister the device, leading to a kernel
panic when sysfs attempts to access the attributes to remove them.
This patch moves the kfree calls to clean up the dynamic attributes to
the thermal_class's thermal_zone_device release function.
Cc: Zhang Rui <rui.zh...@intel.com>
Cc: Eduardo Valentin <edubez...@gmail.com>
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Jacob von Chorus <jacobvoncho...@cwphoto.ca>
---
drivers/thermal/thermal_core.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 641faab..6555913 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -799,6 +799,11 @@ static void thermal_release(struct device *dev)
if (!strncmp(dev_name(dev), "thermal_zone",
sizeof("thermal_zone") - 1)) {
tz = to_thermal_zone(dev);
+ kfree(tz->trip_type_attrs);
+ kfree(tz->trip_temp_attrs);
+ kfree(tz->trip_hyst_attrs);
+ kfree(tz->trips_attribute_group.attrs);
+ kfree(tz->device.groups);
kfree(tz);
} else if (!strncmp(dev_name(dev), "cooling_device",
sizeof("cooling_device") - 1)) {
@@ -1305,10 +1310,6 @@ void thermal_zone_device_unregister(struct
thermal_zone_device *tz)
thermal_zone_device_set_polling(tz, 0);
- kfree(tz->trip_type_attrs);
- kfree(tz->trip_temp_attrs);
- kfree(tz->trip_hyst_attrs);
- kfree(tz->trips_attribute_group.attrs);
thermal_set_governor(tz, NULL);
thermal_remove_hwmon_sysfs(tz);
@@ -1316,7 +1317,6 @@ void thermal_zone_device_unregister(struct
thermal_zone_device *tz)
idr_destroy(>idr);
mutex_destroy(>lock);
device_unregister(>device);
- kfree(tz->device.groups);
}
EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
--
2.9.3
[PATCH] thermal: core: move tz->device.groups cleanup to thermal_release
The device_unregister call in thermal_zone_device_unregister causes the
thermal_zone_device structure to be freed before the call to free the
dynamically allocated attribute groups. This leads to a kernel panic.
Furthermore, the 4 calls to free the trip point attribute structures
occur before the call to unregister the device, leading to a kernel
panic when sysfs attempts to access the attributes to remove them.
This patch moves the kfree calls to clean up the dynamic attributes to
the thermal_class's thermal_zone_device release function.
Cc: Zhang Rui
Cc: Eduardo Valentin
Cc: linux...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Jacob von Chorus
---
drivers/thermal/thermal_core.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 641faab..6555913 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -799,6 +799,11 @@ static void thermal_release(struct device *dev)
if (!strncmp(dev_name(dev), "thermal_zone",
sizeof("thermal_zone") - 1)) {
tz = to_thermal_zone(dev);
+ kfree(tz->trip_type_attrs);
+ kfree(tz->trip_temp_attrs);
+ kfree(tz->trip_hyst_attrs);
+ kfree(tz->trips_attribute_group.attrs);
+ kfree(tz->device.groups);
kfree(tz);
} else if (!strncmp(dev_name(dev), "cooling_device",
sizeof("cooling_device") - 1)) {
@@ -1305,10 +1310,6 @@ void thermal_zone_device_unregister(struct
thermal_zone_device *tz)
thermal_zone_device_set_polling(tz, 0);
- kfree(tz->trip_type_attrs);
- kfree(tz->trip_temp_attrs);
- kfree(tz->trip_hyst_attrs);
- kfree(tz->trips_attribute_group.attrs);
thermal_set_governor(tz, NULL);
thermal_remove_hwmon_sysfs(tz);
@@ -1316,7 +1317,6 @@ void thermal_zone_device_unregister(struct
thermal_zone_device *tz)
idr_destroy(>idr);
mutex_destroy(>lock);
device_unregister(>device);
- kfree(tz->device.groups);
}
EXPORT_SYMBOL_GPL(thermal_zone_device_unregister);
--
2.9.3
