[U-Boot] [PATCH 1/2] gadget: f_thor: fix filename overflow
The thor sender can send filename without null character and it is used without consideration of overflow. Actually, character array for filename is assigned with DEFINE_CACHE_ALIGN_BUFFER() and it is bigger than size of memcpy, so there was no real overflow. Fix filename overflow for code level integrity.
Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com --- drivers/usb/gadget/f_thor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c index f874509..6d38cb6 100644 --- a/drivers/usb/gadget/f_thor.c +++ b/drivers/usb/gadget/f_thor.c @@ -47,7 +47,7 @@ DEFINE_CACHE_ALIGN_BUFFER(unsigned char, thor_rx_data_buf, /* ********************************************************** */ /* THOR protocol - transmission handling */ /* ********************************************************** */ -DEFINE_CACHE_ALIGN_BUFFER(char, f_name, F_NAME_BUF_SIZE); +DEFINE_CACHE_ALIGN_BUFFER(char, f_name, F_NAME_BUF_SIZE + 1); static unsigned long long int thor_file_size; static int alt_setting_num;
@@ -276,6 +276,7 @@ static long long int process_rqt_download(const struct rqt_box *rqt)
thor_file_size = rqt->int_data[1]; memcpy(f_name, rqt->str_data[0], F_NAME_BUF_SIZE); + f_name[F_NAME_BUF_SIZE] = '\0';
debug("INFO: name(%s, %d), size(%llu), type(%d)\n", f_name, 0, thor_file_size, file_type);
During file download, it only uses 32bit variable for file size and it limits maximum file size less than 4GB. Update to support more than 4GB file with using two 32bit variables for file size as thor protocol 5.0.
Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com --- drivers/usb/gadget/f_thor.c | 10 +++++++--- drivers/usb/gadget/f_thor.h | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c index 6d38cb6..c8eda05 100644 --- a/drivers/usb/gadget/f_thor.c +++ b/drivers/usb/gadget/f_thor.c @@ -262,8 +262,10 @@ static long long int process_rqt_download(const struct rqt_box *rqt)
switch (rqt->rqt_data) { case RQT_DL_INIT: - thor_file_size = rqt->int_data[0]; - debug("INIT: total %d bytes\n", rqt->int_data[0]); + thor_file_size = (unsigned long long int)rqt->int_data[0] + + (((unsigned long long int)rqt->int_data[1]) + << 32); + debug("INIT: total %llu bytes\n", thor_file_size); break; case RQT_DL_FILE_INFO: file_type = rqt->int_data[0]; @@ -274,7 +276,9 @@ static long long int process_rqt_download(const struct rqt_box *rqt) break; }
- thor_file_size = rqt->int_data[1]; + thor_file_size = (unsigned long long int)rqt->int_data[1] + + (((unsigned long long int)rqt->int_data[2]) + << 32); memcpy(f_name, rqt->str_data[0], F_NAME_BUF_SIZE); f_name[F_NAME_BUF_SIZE] = '\0';
diff --git a/drivers/usb/gadget/f_thor.h b/drivers/usb/gadget/f_thor.h index 47abc8a..8ba3fa2 100644 --- a/drivers/usb/gadget/f_thor.h +++ b/drivers/usb/gadget/f_thor.h @@ -34,7 +34,7 @@ struct usb_cdc_attribute_vendor_descriptor { __u8 DAUValue; } __packed;
-#define VER_PROTOCOL_MAJOR 4 +#define VER_PROTOCOL_MAJOR 5 #define VER_PROTOCOL_MINOR 0
enum rqt {
Hi Seung-Woo,
During file download, it only uses 32bit variable for file size and it limits maximum file size less than 4GB. Update to support more than 4GB file with using two 32bit variables for file size as thor protocol 5.0.
I assume that it was also tested that this patch will not break devices already using protocol version 4 (like some hobbysts trats2 users, or odroid XU3)?
To be more specific - is the init_data[1] zeroed in the earlier version (version 4) of the THOR protocol?
Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com
drivers/usb/gadget/f_thor.c | 10 +++++++--- drivers/usb/gadget/f_thor.h | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c index 6d38cb6..c8eda05 100644 --- a/drivers/usb/gadget/f_thor.c +++ b/drivers/usb/gadget/f_thor.c @@ -262,8 +262,10 @@ static long long int process_rqt_download(const struct rqt_box *rqt) switch (rqt->rqt_data) { case RQT_DL_INIT:
thor_file_size = rqt->int_data[0];debug("INIT: total %d bytes\n", rqt->int_data[0]);
thor_file_size = (unsigned long longint)rqt->int_data[0] +
(((unsigned long longint)rqt->int_data[1])
<< 32);debug("INIT: total %llu bytes\n", thor_file_size);@@ -274,7 +276,9 @@ static long long int process_rqt_download(const struct rqt_box *rqt) break; }
thor_file_size = rqt->int_data[1];
thor_file_size = (unsigned long longint)rqt->int_data[1] +
(((unsigned long longint)rqt->int_data[2])
<< 32);diff --git a/drivers/usb/gadget/f_thor.h b/drivers/usb/gadget/f_thor.h index 47abc8a..8ba3fa2 100644 --- a/drivers/usb/gadget/f_thor.h +++ b/drivers/usb/gadget/f_thor.h @@ -34,7 +34,7 @@ struct usb_cdc_attribute_vendor_descriptor { __u8 DAUValue; } __packed;
-#define VER_PROTOCOL_MAJOR 4 +#define VER_PROTOCOL_MAJOR 5 #define VER_PROTOCOL_MINOR 0
enum rqt {
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Hello Lukasz,
On 2018년 05월 10일 19:58, Lukasz Majewski wrote:
Hi Seung-Woo,
During file download, it only uses 32bit variable for file size and it limits maximum file size less than 4GB. Update to support more than 4GB file with using two 32bit variables for file size as thor protocol 5.0.
I assume that it was also tested that this patch will not break devices already using protocol version 4 (like some hobbysts trats2 users, or odroid XU3)?
Yes, of course. I have checked all those devices are using THOR protocol version 4.0 and tested with the devices.
To be more specific - is the init_data[1] zeroed in the earlier version (version 4) of the THOR protocol?
From the thor tool like Tizen lthor[1], it clears all request.
And THOR protocol 5.0 support has also done from Tizen lthor and it is currently under review. I have tested THOR protocol 5.0 device with THOR protocol 4.0 lthor and reserve way also. From lthor with THOR protocol 5.0, I just added to check protocol version of target and if the target protocol version is less than 5.0 than I made lthor refuse 4GB or larger file.
[1]: https://git.tizen.org/cgit/tools/lthor
Best Regards, - Seung-Woo Kim
Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com
drivers/usb/gadget/f_thor.c | 10 +++++++--- drivers/usb/gadget/f_thor.h | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/f_thor.c b/drivers/usb/gadget/f_thor.c index 6d38cb6..c8eda05 100644 --- a/drivers/usb/gadget/f_thor.c +++ b/drivers/usb/gadget/f_thor.c @@ -262,8 +262,10 @@ static long long int process_rqt_download(const struct rqt_box *rqt) switch (rqt->rqt_data) { case RQT_DL_INIT:
thor_file_size = rqt->int_data[0];debug("INIT: total %d bytes\n", rqt->int_data[0]);
thor_file_size = (unsigned long longint)rqt->int_data[0] +
(((unsigned long longint)rqt->int_data[1])
<< 32);debug("INIT: total %llu bytes\n", thor_file_size);@@ -274,7 +276,9 @@ static long long int process_rqt_download(const struct rqt_box *rqt) break; }
thor_file_size = rqt->int_data[1];
thor_file_size = (unsigned long longint)rqt->int_data[1] +
(((unsigned long longint)rqt->int_data[2])
<< 32);diff --git a/drivers/usb/gadget/f_thor.h b/drivers/usb/gadget/f_thor.h index 47abc8a..8ba3fa2 100644 --- a/drivers/usb/gadget/f_thor.h +++ b/drivers/usb/gadget/f_thor.h @@ -34,7 +34,7 @@ struct usb_cdc_attribute_vendor_descriptor { __u8 DAUValue; } __packed;
-#define VER_PROTOCOL_MAJOR 4 +#define VER_PROTOCOL_MAJOR 5 #define VER_PROTOCOL_MINOR 0
enum rqt {
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
participants (2)
-
Lukasz Majewski -
Seung-Woo Kim