Re: [PATCH 3/4] net: usb: rtl8150: use usb_control_msg_recv() and usb_control_msg_send()

Oliver Neukum
2020-09-23T10:23:00Z

Am Mittwoch, den 23.09.2020, 14:35 +0530 schrieb Himadri Pandya:

Hi,

> Many usage of usb_control_msg() do not have proper error check on return
> value leaving scope for bugs on short reads. New usb_control_msg_recv()
> and usb_control_msg_send() nicely wraps usb_control_msg() with proper
> error check. Hence use the wrappers instead of calling usb_control_msg()
> directly.
>
> Signed-off-by: Himadri Pandya <himadrispandya@gmail.com>
Nacked-by: Oliver Neukum <oneukum@suse.com>

> ---
> drivers/net/usb/rtl8150.c | 32 ++++++--------------------------
> 1 file changed, 6 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 733f120c852b..e3002b675921 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -152,36 +152,16 @@ static const char driver_name [] = "rtl8150";
> */
> static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
> {
> - void *buf;
> - int ret;
> -
> - buf = kmalloc(size, GFP_NOIO);

GFP_NOIO is used here for a reason. You need to use this helper
while in contexts of error recovery and runtime PM.

> - if (!buf)
> - return -ENOMEM;
> -
> - ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
> - RTL8150_REQ_GET_REGS, RTL8150_REQT_READ,
> - indx, 0, buf, size, 500);
> - if (ret > 0 && ret <= size)
> - memcpy(data, buf, ret);
> - kfree(buf);
> - return ret;
> + return usb_control_msg_recv(dev->udev, 0, RTL8150_REQ_GET_REGS,
> + RTL8150_REQT_READ, indx, 0, data,
> + size, 500);

This internally uses kmemdup() with GFP_KERNEL.
You cannot make this change. The API does not support it.
I am afraid we will have to change the API first, before more
such changes are done.

I would suggest dropping the whole series for now.

Regards
Oliver