| Message ID | 20210524120836.1580468-1-liushixin2@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [-next,media] media: Replaced simple_strtol() with kstrtoint() | expand |
On 24/05/2021 14:08, Liu Shixin wrote: > It looks like that memcpy() is a superfluous operation in > parse_token()/parse_mtoken(). Simple these two functions and > use kstrtoint() instead of simple_strtol() to avoid data > overflow. Always mention which driver you are changing in the subject line. Just saying 'media:' suggests a subsystem-wide patch, but this is just for a single driver. Regards, Hans > > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > --- > drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 25 ++---------------------- > 1 file changed, 2 insertions(+), 23 deletions(-) > > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > index 9f71d8c2a3c6..8ae3ad80cccb 100644 > --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > @@ -355,11 +355,8 @@ static int parse_token(const char *ptr,unsigned int len, > int *valptr, > const char * const *names, unsigned int namecnt) > { > - char buf[33]; > unsigned int slen; > unsigned int idx; > - int negfl; > - char *p2; > *valptr = 0; > if (!names) namecnt = 0; > for (idx = 0; idx < namecnt; idx++) { > @@ -370,18 +367,7 @@ static int parse_token(const char *ptr,unsigned int len, > *valptr = idx; > return 0; > } > - negfl = 0; > - if ((*ptr == '-') || (*ptr == '+')) { > - negfl = (*ptr == '-'); > - ptr++; len--; > - } > - if (len >= sizeof(buf)) return -EINVAL; > - memcpy(buf,ptr,len); > - buf[len] = 0; > - *valptr = simple_strtol(buf,&p2,0); > - if (negfl) *valptr = -(*valptr); > - if (*p2) return -EINVAL; > - return 1; > + return kstrtoint(ptr, 0, valptr) ? -EINVAL : 1; > } > > > @@ -389,10 +375,8 @@ static int parse_mtoken(const char *ptr,unsigned int len, > int *valptr, > const char **names,int valid_bits) > { > - char buf[33]; > unsigned int slen; > unsigned int idx; > - char *p2; > int msk; > *valptr = 0; > for (idx = 0, msk = 1; valid_bits; idx++, msk <<= 1) { > @@ -405,12 +389,7 @@ static int parse_mtoken(const char *ptr,unsigned int len, > *valptr = msk; > return 0; > } > - if (len >= sizeof(buf)) return -EINVAL; > - memcpy(buf,ptr,len); > - buf[len] = 0; > - *valptr = simple_strtol(buf,&p2,0); > - if (*p2) return -EINVAL; > - return 0; > + return kstrtoint(ptr, 0, valptr); > } > > >
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c index 9f71d8c2a3c6..8ae3ad80cccb 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c @@ -355,11 +355,8 @@ static int parse_token(const char *ptr,unsigned int len, int *valptr, const char * const *names, unsigned int namecnt) { - char buf[33]; unsigned int slen; unsigned int idx; - int negfl; - char *p2; *valptr = 0; if (!names) namecnt = 0; for (idx = 0; idx < namecnt; idx++) { @@ -370,18 +367,7 @@ static int parse_token(const char *ptr,unsigned int len, *valptr = idx; return 0; } - negfl = 0; - if ((*ptr == '-') || (*ptr == '+')) { - negfl = (*ptr == '-'); - ptr++; len--; - } - if (len >= sizeof(buf)) return -EINVAL; - memcpy(buf,ptr,len); - buf[len] = 0; - *valptr = simple_strtol(buf,&p2,0); - if (negfl) *valptr = -(*valptr); - if (*p2) return -EINVAL; - return 1; + return kstrtoint(ptr, 0, valptr) ? -EINVAL : 1; } @@ -389,10 +375,8 @@ static int parse_mtoken(const char *ptr,unsigned int len, int *valptr, const char **names,int valid_bits) { - char buf[33]; unsigned int slen; unsigned int idx; - char *p2; int msk; *valptr = 0; for (idx = 0, msk = 1; valid_bits; idx++, msk <<= 1) { @@ -405,12 +389,7 @@ static int parse_mtoken(const char *ptr,unsigned int len, *valptr = msk; return 0; } - if (len >= sizeof(buf)) return -EINVAL; - memcpy(buf,ptr,len); - buf[len] = 0; - *valptr = simple_strtol(buf,&p2,0); - if (*p2) return -EINVAL; - return 0; + return kstrtoint(ptr, 0, valptr); }
It looks like that memcpy() is a superfluous operation in parse_token()/parse_mtoken(). Simple these two functions and use kstrtoint() instead of simple_strtol() to avoid data overflow. Signed-off-by: Liu Shixin <liushixin2@huawei.com> --- drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 25 ++---------------------- 1 file changed, 2 insertions(+), 23 deletions(-)