Skip to contentSkip to conversation

usb: gadget: f_fs: Use struct_size() and flex_array_size() helpers

PATCH In review Gustavo A. R. Silva 2 participants 2 messages Jan 20, 2022 USB

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 usb: gadget: f_fs: Use struct_size() and flex_array_size() helpers

Make use of the struct_size() and flex_array_size() helpers instead of an open-coded version, in order to avoid any potential type mistakes or integer overflows that, in the worst scenario, could lead to heap overflows.

Also, address the following sparse warnings:
drivers/usb/gadget/function/f_fs.c:922:23: warning: using sizeof on a flexible structure

Link https://github.com/KSPP/linux/issues/174
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/gadget/function/f_fs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_fs.c +2 -2
index 25ad1e97a458..7461d27e9604 100644
@@ -919,12 +919,12 @@ static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile,
 		data_len, ret);
 
 	data_len -= ret;
-	buf = kmalloc(sizeof(*buf) + data_len, GFP_KERNEL);
+	buf = kmalloc(struct_size(buf, storage, data_len), GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 	buf->length = data_len;
 	buf->data = buf->storage;
-	memcpy(buf->storage, data + ret, data_len);
+	memcpy(buf->storage, data + ret, flex_array_size(buf, storage, data_len));
 
 	/*
 	 * At this point read_buffer is NULL or READ_BUFFER_DROP (if
Signature · 1 line
2.27.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 20 lines · Gustavo A. R. Silva

Make use of the struct_size() and flex_array_size() helpers instead of an open-coded version, in order to avoid any potential type mistakes or integer overflows that, in the worst scenario, could lead to heap overflows.

Also, address the following sparse warnings:
drivers/usb/gadget/function/f_fs.c:922:23: warning: using sizeof on a flexible structure

Link https://github.com/KSPP/linux/issues/174
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/gadget/function/f_fs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/usb/gadget/function/f_fs.c +0 -0
index 25ad1e97a458..7461d27e9604 100644
@@ -919,12 +919,12 @@ static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile,
 		data_len, ret);

drivers/usb/gadget/function/f_fs.c static ssize_t __ffs_epfile_read_data(struct ffs_epfile *epfile, · line 927

Quoted 10 lines
drivers/usb/gadget/function/f_fs.c +2 -2
 
 	data_len -= ret;
-	buf = kmalloc(sizeof(*buf) + data_len, GFP_KERNEL);
+	buf = kmalloc(struct_size(buf, storage, data_len), GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 	buf->length = data_len;
 	buf->data = buf->storage;
-	memcpy(buf->storage, data + ret, data_len);
+	memcpy(buf->storage, data + ret, flex_array_size(buf, storage, data_len));

Looks good to me. This is a place where I think we'll be back later to fix up the alloc/deserialize-from-memory pattern with a future helper, but in the meantime, yes, let's get this covered by struct_size().

Reviewed-by Kees Cook <[email protected]>
Quoted 6 lines
drivers/usb/gadget/function/f_fs.c +0 -0
 	/*
 	 * At this point read_buffer is NULL or READ_BUFFER_DROP (if
Signature · 1 line
2.27.0
Signature · 1 line
Kees Cook