Re: [PATCH] usb: misc: cypress_cy7c63: check control transfer status

[Keshav Verma]

read_port() currently ignores errors from vendor_command() and always
returns the cached port value. This can report stale data when the USB
control transfer fails or returns a short response.

Convert vendor_command() to usb_control_msg_recv(), which returns 0 on
success and a negative errno on failure, and propagate transport errors
from read_port() instead of returning cached data.

Fixes: 9189bfc2df0f ("[PATCH] USB: rename Cypress CY7C63xxx driver to
proper name and fix up some tiny things")
Signed-off-by: Keshav Verma <iganschel@xxxxxxxxx>
---
v2:
- Use usb_control_msg_recv() in vendor_command().
- Remove the obsolete short-transfer check.

 drivers/usb/misc/cypress_cy7c63.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/misc/cypress_cy7c63.c
b/drivers/usb/misc/cypress_cy7c63.c
index 4a7f955ba85b..47cb05c3a870 100644
--- a/drivers/usb/misc/cypress_cy7c63.c
+++ b/drivers/usb/misc/cypress_cy7c63.c
@@ -69,8 +69,7 @@ struct cypress {
 static int vendor_command(struct cypress *dev, unsigned char request,
                          unsigned char address, unsigned char data)
 {
-       int retval = 0;
-       unsigned int pipe;
+       int retval;
        unsigned char *iobuf;

        /* allocate some memory for the i/o buffer*/
@@ -80,17 +79,19 @@ static int vendor_command(struct cypress *dev,
unsigned char request,
                goto error;
        }

-       dev_dbg(&dev->udev->dev, "Sending usb_control_msg (data: %d)\n", data);
-
-       /* prepare usb control message and send it upstream */
-       pipe = usb_rcvctrlpipe(dev->udev, 0);
-       retval = usb_control_msg(dev->udev, pipe, request,
-                                USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
-                                address, data, iobuf, CYPRESS_MAX_REQSIZE,
-                                USB_CTRL_GET_TIMEOUT);
-       /* we must not process garbage */
-       if (retval < 2)
+       dev_dbg(&dev->udev->dev, "Sending usb_control_msg_recv (data:
%d)\n", data);
+
+       retval = usb_control_msg_recv(dev->udev, 0, request,
+                               USB_DIR_IN | USB_TYPE_VENDOR |
+                               USB_RECIP_OTHER,
+                               address, data, iobuf,
+                               CYPRESS_MAX_REQSIZE,
+                               USB_CTRL_GET_TIMEOUT,
+                               GFP_KERNEL);
+       if (retval < 0) {
+               dev_err(&dev->udev->dev, "usb_control_msg_recv failed:
%d\n", retval);
                goto err_buf;
+       }

        /* store returned data (more READs to be added) */
        switch (request) {
@@ -177,6 +178,8 @@ static ssize_t read_port(struct device *dev,
struct device_attribute *attr,
        result = vendor_command(cyp, CYPRESS_READ_PORT, read_id, 0);

        dev_dbg(&cyp->udev->dev, "Result of vendor_command: %d\n\n", result);
+       if (result < 0)
+               return result;

        return sprintf(buf, "%d", cyp->port[port_num]);
 }
--
2.39.5