* [U-Boot] [PATCH 1/2] gadget: f_thor: fix filename overflow [not found] <CGME20180510015213epcas1p1ecf6315ed1e762c0d51338137c658425@epcas1p1.samsung.com> @ 2018-05-10 1:52 ` Seung-Woo Kim 2018-05-10 1:52 ` [U-Boot] [PATCH 2/2] gadget: f_thor: update to support more than 4GB file as thor 5.0 Seung-Woo Kim 0 siblings, 1 reply; 4+ messages in thread From: Seung-Woo Kim @ 2018-05-10 1:52 UTC (permalink / raw) To: u-boot 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); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH 2/2] gadget: f_thor: update to support more than 4GB file as thor 5.0 2018-05-10 1:52 ` [U-Boot] [PATCH 1/2] gadget: f_thor: fix filename overflow Seung-Woo Kim @ 2018-05-10 1:52 ` Seung-Woo Kim 2018-05-10 10:58 ` Lukasz Majewski 0 siblings, 1 reply; 4+ messages in thread From: Seung-Woo Kim @ 2018-05-10 1:52 UTC (permalink / raw) To: u-boot 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 { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH 2/2] gadget: f_thor: update to support more than 4GB file as thor 5.0 2018-05-10 1:52 ` [U-Boot] [PATCH 2/2] gadget: f_thor: update to support more than 4GB file as thor 5.0 Seung-Woo Kim @ 2018-05-10 10:58 ` Lukasz Majewski 2018-05-11 0:48 ` Seung-Woo Kim 0 siblings, 1 reply; 4+ messages in thread From: Lukasz Majewski @ 2018-05-10 10:58 UTC (permalink / raw) To: u-boot 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 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 { 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 at denx.de -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 488 bytes Desc: OpenPGP digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180510/3d50e8e3/attachment.sig> ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH 2/2] gadget: f_thor: update to support more than 4GB file as thor 5.0 2018-05-10 10:58 ` Lukasz Majewski @ 2018-05-11 0:48 ` Seung-Woo Kim 0 siblings, 0 replies; 4+ messages in thread From: Seung-Woo Kim @ 2018-05-11 0:48 UTC (permalink / raw) To: u-boot 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 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 { > > 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 at denx.de > -- Seung-Woo Kim Samsung Research -- ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-11 0:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20180510015213epcas1p1ecf6315ed1e762c0d51338137c658425@epcas1p1.samsung.com>
2018-05-10 1:52 ` [U-Boot] [PATCH 1/2] gadget: f_thor: fix filename overflow Seung-Woo Kim
2018-05-10 1:52 ` [U-Boot] [PATCH 2/2] gadget: f_thor: update to support more than 4GB file as thor 5.0 Seung-Woo Kim
2018-05-10 10:58 ` Lukasz Majewski
2018-05-11 0:48 ` Seung-Woo Kim
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.