Re: [PATCH -next] USB: core: Switch to use kmemdup_nul() helper

From: Greg KH <gregkh@linuxfoundation.org>
To: Ruan Jinjie <ruanjinjie@huawei.com>
Cc: hadess@hadess.net, benjamin.tissoires@redhat.com,
	herve.codina@bootlin.com, robh@kernel.org,
	mailhol.vincent@wanadoo.fr, linux-usb@vger.kernel.org
Subject: Re: [PATCH -next] USB: core: Switch to use kmemdup_nul() helper
Date: Tue, 8 Aug 2023 10:22:14 +0200	[thread overview]
Message-ID: <2023080822-embark-quaking-074e@gregkh> (raw)
In-Reply-To: <20230807124610.2283583-1-ruanjinjie@huawei.com>

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