lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 23 Sep 2020 12:22:56 +0200 From: Greg KH <gregkh@...uxfoundation.org> To: Himadri Pandya <himadrispandya@...il.com> Cc: davem@...emloft.net, kuba@...nel.org, oneukum@...e.com, pankaj.laxminarayan.bharadiya@...el.com, keescook@...omium.org, yuehaibing@...wei.com, petkan@...leusys.com, ogiannou@...il.com, linux-usb@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linuxfoundation.org Subject: Re: [PATCH 4/4] net: rndis_host: use usb_control_msg_recv() and usb_control_msg_send() On Wed, Sep 23, 2020 at 02:35:19PM +0530, Himadri Pandya wrote: > The 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@...il.com> > --- > drivers/net/usb/rndis_host.c | 44 ++++++++++++++---------------------- > 1 file changed, 17 insertions(+), 27 deletions(-) > > diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c > index 6fa7a009a24a..30fc4a7183d3 100644 > --- a/drivers/net/usb/rndis_host.c > +++ b/drivers/net/usb/rndis_host.c > @@ -113,14 +113,13 @@ int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen) > buf->request_id = (__force __le32) xid; > } > master_ifnum = info->control->cur_altsetting->desc.bInterfaceNumber; > - retval = usb_control_msg(dev->udev, > - usb_sndctrlpipe(dev->udev, 0), > - USB_CDC_SEND_ENCAPSULATED_COMMAND, > - USB_TYPE_CLASS | USB_RECIP_INTERFACE, > - 0, master_ifnum, > - buf, le32_to_cpu(buf->msg_len), > - RNDIS_CONTROL_TIMEOUT_MS); > - if (unlikely(retval < 0 || xid == 0)) > + retval = usb_control_msg_send(dev->udev, 0, > + USB_CDC_SEND_ENCAPSULATED_COMMAND, > + USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + 0, master_ifnum, buf, > + le32_to_cpu(buf->msg_len), > + RNDIS_CONTROL_TIMEOUT_MS); > + if (unlikely(xid == 0)) > return retval; > > /* Some devices don't respond on the control channel until > @@ -139,14 +138,11 @@ int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen) > /* Poll the control channel; the request probably completed immediately */ > rsp = le32_to_cpu(buf->msg_type) | RNDIS_MSG_COMPLETION; > for (count = 0; count < 10; count++) { > - memset(buf, 0, CONTROL_BUFFER_SIZE); > - retval = usb_control_msg(dev->udev, > - usb_rcvctrlpipe(dev->udev, 0), > - USB_CDC_GET_ENCAPSULATED_RESPONSE, > - USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, > - 0, master_ifnum, > - buf, buflen, > - RNDIS_CONTROL_TIMEOUT_MS); > + retval = usb_control_msg_recv(dev->udev, 0, > + USB_CDC_GET_ENCAPSULATED_RESPONSE, > + USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + 0, master_ifnum, buf, buflen, > + RNDIS_CONTROL_TIMEOUT_MS); > if (likely(retval >= 8)) { retval here is never going to be positive, right? So I don't think this patch is correct :( > msg_type = le32_to_cpu(buf->msg_type); > msg_len = le32_to_cpu(buf->msg_len); > @@ -178,17 +174,11 @@ int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen) > msg->msg_type = cpu_to_le32(RNDIS_MSG_KEEPALIVE_C); > msg->msg_len = cpu_to_le32(sizeof *msg); > msg->status = cpu_to_le32(RNDIS_STATUS_SUCCESS); > - retval = usb_control_msg(dev->udev, > - usb_sndctrlpipe(dev->udev, 0), > - USB_CDC_SEND_ENCAPSULATED_COMMAND, > - USB_TYPE_CLASS | USB_RECIP_INTERFACE, > - 0, master_ifnum, > - msg, sizeof *msg, > - RNDIS_CONTROL_TIMEOUT_MS); > - if (unlikely(retval < 0)) > - dev_dbg(&info->control->dev, > - "rndis keepalive err %d\n", > - retval); > + retval = usb_control_msg_send(dev->udev, 0, > + USB_CDC_SEND_ENCAPSULATED_COMMAND, > + USB_TYPE_CLASS | USB_RECIP_INTERFACE, > + 0, master_ifnum, msg, sizeof(*msg), > + RNDIS_CONTROL_TIMEOUT_MS); You lost the error message that the previous call had if something went wrong. Don't know if it's really needed, but there's no reason to remove it here. thanks, greg k-h
Powered by blists - more mailing lists