| Message ID | 20230807124610.2283583-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] USB: core: Switch to use kmemdup_nul() helper | expand |
On Mon, Aug 07, 2023 at 08:46:10PM +0800, Ruan Jinjie wrote: > Use kmemdup_nul() helper instead of open-coding it to simplify the code. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > drivers/usb/core/message.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > index 0d2bfc909019..5762fd04f0d5 100644 > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -1029,10 +1029,9 @@ char *usb_cache_string(struct usb_device *udev, int index) > if (buf) { > len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE); > if (len > 0) { > - smallbuf = kmalloc(++len, GFP_NOIO); > + smallbuf = kmemdup_nul(buf, len, GFP_NOIO); > if (!smallbuf) > return buf; > - memcpy(smallbuf, buf, len); But you changed the logic here, you now added an extra \0 where the existing code did not. Are you sure you mean to do this? If so, why, and it needs to be documented in the changelog text. What this could be is a call to kmemdup() if you really want it, but be careful about the ++len usage... Also, does this need to be changed at all? How was it tested? thanks, greg k-h
On 2023/8/8 16:22, Greg KH wrote: > On Mon, Aug 07, 2023 at 08:46:10PM +0800, Ruan Jinjie wrote: >> Use kmemdup_nul() helper instead of open-coding it to simplify the code. >> >> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> >> --- >> drivers/usb/core/message.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c >> index 0d2bfc909019..5762fd04f0d5 100644 >> --- a/drivers/usb/core/message.c >> +++ b/drivers/usb/core/message.c >> @@ -1029,10 +1029,9 @@ char *usb_cache_string(struct usb_device *udev, int index) >> if (buf) { >> len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE); >> if (len > 0) { >> - smallbuf = kmalloc(++len, GFP_NOIO); >> + smallbuf = kmemdup_nul(buf, len, GFP_NOIO); >> if (!smallbuf) >> return buf; >> - memcpy(smallbuf, buf, len); > > But you changed the logic here, you now added an extra \0 where the > existing code did not. Are you sure you mean to do this? If so, why, > and it needs to be documented in the changelog text. Right! There is a problem because of the ++len, and the logic has been changed. Sorry, I'll carefully check the patches issued in the future. > > What this could be is a call to kmemdup() if you really want it, but be > careful about the ++len usage... > > Also, does this need to be changed at all? How was it tested? It's best to keep it as it is. Sorry, just walk through the code. > > thanks, > > greg k-h
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 0d2bfc909019..5762fd04f0d5 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1029,10 +1029,9 @@ char *usb_cache_string(struct usb_device *udev, int index) if (buf) { len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE); if (len > 0) { - smallbuf = kmalloc(++len, GFP_NOIO); + smallbuf = kmemdup_nul(buf, len, GFP_NOIO); if (!smallbuf) return buf; - memcpy(smallbuf, buf, len); } kfree(buf); }
Use kmemdup_nul() helper instead of open-coding it to simplify the code. Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- drivers/usb/core/message.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)