Message ID | 20201031213533.40829-1-anant.thazhemadam@gmail.com |
---|---|
State | Superseded |
Delegated to: | David Miller |
Headers | show |
Series | [v3] net: usb: usbnet: update __usbnet_{read|write}_cmd() to use new API | expand |
Context | Check | Description |
---|---|---|
jkicinski/cover_letter | success | Link |
jkicinski/fixes_present | success | Link |
jkicinski/patch_count | success | Link |
jkicinski/tree_selection | success | Guessed tree name to be net-next |
jkicinski/subject_prefix | warning | Target tree name not specified in the subject |
jkicinski/source_inline | success | Was 0 now: 0 |
jkicinski/verify_signedoff | success | Link |
jkicinski/module_param | success | Was 0 now: 0 |
jkicinski/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
jkicinski/verify_fixes | success | Link |
jkicinski/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 74 lines checked |
jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
jkicinski/header_inline | success | Link |
jkicinski/stable | success | Stable not CCed |
Am Sonntag, den 01.11.2020, 03:05 +0530 schrieb Anant Thazhemadam: > Currently, __usbnet_{read|write}_cmd() use usb_control_msg(). > However, this could lead to potential partial reads/writes being > considered valid, and since most of the callers of > usbnet_{read|write}_cmd() don't take partial reads/writes into account > (only checking for negative error number is done), and this can lead to > issues. > Hi, plesae send this as a post of its own. We cannot take a new set as a reply to an older set. Regards Oliver
On 02/11/20 3:10 pm, Oliver Neukum wrote: > Am Sonntag, den 01.11.2020, 03:05 +0530 schrieb Anant Thazhemadam: >> Currently, __usbnet_{read|write}_cmd() use usb_control_msg(). >> However, this could lead to potential partial reads/writes being >> considered valid, and since most of the callers of >> usbnet_{read|write}_cmd() don't take partial reads/writes into account >> (only checking for negative error number is done), and this can lead to >> issues. >> > Hi, > > plesae send this as a post of its own. We cannot take a new set > as a reply to an older set. > > Regards > Oliver > Got it. I will do that. Thanks, Anant
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index bf6c58240bd4..b2df3417a41c 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1982,64 +1982,32 @@ EXPORT_SYMBOL(usbnet_link_change); static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype, u16 value, u16 index, void *data, u16 size) { - void *buf = NULL; - int err = -ENOMEM; netdev_dbg(dev->net, "usbnet_read_cmd cmd=0x%02x reqtype=%02x" " value=0x%04x index=0x%04x size=%d\n", cmd, reqtype, value, index, size); - if (size) { - buf = kmalloc(size, GFP_KERNEL); - if (!buf) - goto out; - } - - err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0), - cmd, reqtype, value, index, buf, size, - USB_CTRL_GET_TIMEOUT); - if (err > 0 && err <= size) { - if (data) - memcpy(data, buf, err); - else - netdev_dbg(dev->net, - "Huh? Data requested but thrown away.\n"); - } - kfree(buf); -out: - return err; + return usb_control_msg_recv(dev->udev, 0, + cmd, reqtype, value, index, data, size, + USB_CTRL_GET_TIMEOUT, GFP_KERNEL); } static int __usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype, u16 value, u16 index, const void *data, u16 size) { - void *buf = NULL; - int err = -ENOMEM; - netdev_dbg(dev->net, "usbnet_write_cmd cmd=0x%02x reqtype=%02x" " value=0x%04x index=0x%04x size=%d\n", cmd, reqtype, value, index, size); - if (data) { - buf = kmemdup(data, size, GFP_KERNEL); - if (!buf) - goto out; - } else { - if (size) { - WARN_ON_ONCE(1); - err = -EINVAL; - goto out; - } - } - - err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0), - cmd, reqtype, value, index, buf, size, - USB_CTRL_SET_TIMEOUT); - kfree(buf); + if (size && !data) { + WARN_ON_ONCE(1); + return -EINVAL; + } -out: - return err; + return usb_control_msg_send(dev->udev, 0, + cmd, reqtype, value, index, data, size, + USB_CTRL_SET_TIMEOUT, GFP_KERNEL); } /*
Currently, __usbnet_{read|write}_cmd() use usb_control_msg(). However, this could lead to potential partial reads/writes being considered valid, and since most of the callers of usbnet_{read|write}_cmd() don't take partial reads/writes into account (only checking for negative error number is done), and this can lead to issues. However, the new usb_control_msg_{send|recv}() APIs don't allow partial reads and writes. Using the new APIs also relaxes the return value checking that must be done after usbnet_{read|write}_cmd() is called. Signed-off-by: Anant Thazhemadam <anant.thazhemadam@gmail.com> --- Changes in v3: * Aligned continuation lines after the opening brackets Changes in v2: * Fix build error drivers/net/usb/usbnet.c | 52 ++++++++-------------------------------- 1 file changed, 10 insertions(+), 42 deletions(-)