| Message ID | 20180517170336.8426-6-guido@kiener-muenchen.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 17, 2018 at 07:03:29PM +0200, Guido Kiener wrote: > Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the > control pipe. Used by specific applications of IVI Foundation, > Inc. to implement VISA API functions: viUsbControlIn/Out. > > Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com> > Reviewed-by: Steve Bayless <steve_bayless@keysight.com> > --- > drivers/usb/class/usbtmc.c | 61 ++++++++++++++++++++++++++++++++++++ > include/uapi/linux/usb/tmc.h | 15 +++++++++ > 2 files changed, 76 insertions(+) > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > index 152e2daa9644..00c2e51a23a7 100644 > --- a/drivers/usb/class/usbtmc.c > +++ b/drivers/usb/class/usbtmc.c > @@ -5,6 +5,7 @@ > * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany > * Copyright (C) 2008 Novell, Inc. > * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> > + * Copyright (C) 2018, IVI Foundation, Inc. > */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > @@ -1263,6 +1264,62 @@ static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) > return rv; > } > > +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, > + void __user *arg) > +{ > + struct device *dev = &data->intf->dev; > + struct usbtmc_ctrlrequest request; > + u8 *buffer = NULL; > + int rv; > + unsigned long res; > + > + res = copy_from_user(&request, arg, sizeof(struct usbtmc_ctrlrequest)); > + if (res) > + return -EFAULT; > + > + buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL); No validation that wLength is a sane number? Go to the whiteboard nearby and write a the top of it: ALL INPUT IS EVIL > + if (!buffer) > + return -ENOMEM; > + > + if ((request.req.bRequestType & USB_DIR_IN) == 0 > + && request.req.wLength) { > + res = copy_from_user(buffer, request.data, > + request.req.wLength); > + if (res) { > + rv = -EFAULT; > + goto exit; > + } > + } > + > + rv = usb_control_msg(data->usb_dev, > + usb_rcvctrlpipe(data->usb_dev, 0), > + request.req.bRequest, > + request.req.bRequestType, > + request.req.wValue, > + request.req.wIndex, > + buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT); request.req.wLength might not be the actual size of buffer here due to your above min_t() check. > + > + if (rv < 0) { > + dev_err(dev, "%s failed %d\n", __func__, rv); > + goto exit; > + } > + if ((request.req.bRequestType & USB_DIR_IN)) { > + if (rv > request.req.wLength) { > + dev_warn(dev, "%s returned too much data: %d\n", > + __func__, rv); > + rv = request.req.wLength; Why are you returning a value that is greater than the size asked for? That's going to make userspace confused. Then there is the generic question of "why are you allowing arbitrary USB control messages to be sent to devices" here. That feels like a very odd way for a device that is supposed to be following a standard to be working. You are circumventing the standard here by allowing any message to be sent to the device. While nice for that one type of device, you are breaking interoperability in horrible ways (now apps only work with one type of device.) Why did you all agree to allow this to happen? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Zitat von Greg KH <gregkh@linuxfoundation.org>: > On Thu, May 17, 2018 at 07:03:29PM +0200, Guido Kiener wrote: >> Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the >> control pipe. Used by specific applications of IVI Foundation, >> Inc. to implement VISA API functions: viUsbControlIn/Out. >> >> Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com> >> Reviewed-by: Steve Bayless <steve_bayless@keysight.com> >> --- >> drivers/usb/class/usbtmc.c | 61 ++++++++++++++++++++++++++++++++++++ >> include/uapi/linux/usb/tmc.h | 15 +++++++++ >> 2 files changed, 76 insertions(+) >> >> diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c >> index 152e2daa9644..00c2e51a23a7 100644 >> --- a/drivers/usb/class/usbtmc.c >> +++ b/drivers/usb/class/usbtmc.c >> @@ -5,6 +5,7 @@ >> * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany >> * Copyright (C) 2008 Novell, Inc. >> * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> >> + * Copyright (C) 2018, IVI Foundation, Inc. >> */ >> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> @@ -1263,6 +1264,62 @@ static int >> usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) >> return rv; >> } >> >> +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, >> + void __user *arg) >> +{ >> + struct device *dev = &data->intf->dev; >> + struct usbtmc_ctrlrequest request; >> + u8 *buffer = NULL; >> + int rv; >> + unsigned long res; >> + >> + res = copy_from_user(&request, arg, sizeof(struct usbtmc_ctrlrequest)); >> + if (res) >> + return -EFAULT; >> + >> + buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL); > > No validation that wLength is a sane number? Ooops. It should be buffer = kmalloc(max_t(u16, 256, request.req.wLength), GFP_KERNEL); Then all values of request.req.wLength (0..65535) are ok. > > Go to the whiteboard nearby and write a the top of it: > ALL INPUT IS EVIL > >> + if (!buffer) >> + return -ENOMEM; >> + >> + if ((request.req.bRequestType & USB_DIR_IN) == 0 >> + && request.req.wLength) { >> + res = copy_from_user(buffer, request.data, >> + request.req.wLength); >> + if (res) { >> + rv = -EFAULT; >> + goto exit; >> + } >> + } >> + >> + rv = usb_control_msg(data->usb_dev, >> + usb_rcvctrlpipe(data->usb_dev, 0), >> + request.req.bRequest, >> + request.req.bRequestType, >> + request.req.wValue, >> + request.req.wIndex, >> + buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT); > > request.req.wLength might not be the actual size of buffer here due to > your above min_t() check. > >> + >> + if (rv < 0) { >> + dev_err(dev, "%s failed %d\n", __func__, rv); >> + goto exit; >> + } >> + if ((request.req.bRequestType & USB_DIR_IN)) { >> + if (rv > request.req.wLength) { >> + dev_warn(dev, "%s returned too much data: %d\n", >> + __func__, rv); >> + rv = request.req.wLength; > > Why are you returning a value that is greater than the size asked for? > That's going to make userspace confused. I followed the rule INPUT IS EVIL, but I'm not sure whether this really can happen that a device can return more than requested. I will a have a closer look again. > > Then there is the generic question of "why are you allowing arbitrary > USB control messages to be sent to devices" here. That feels like a > very odd way for a device that is supposed to be following a standard to > be working. You are circumventing the standard here by allowing any > message to be sent to the device. While nice for that one type of > device, you are breaking interoperability in horrible ways (now apps > only work with one type of device.) > > Why did you all agree to allow this to happen? I did not define the USBTMC and VISA standard. However the API requires to send this generic control request. Otherwise we have to use the libusb again. http://zone.ni.com/reference/en-XX/help/370131S-01/ni-visa/viusbcontrolin/ > > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 18, 2018 at 01:32:51PM +0000, guido@kiener-muenchen.de wrote: > > Zitat von Greg KH <gregkh@linuxfoundation.org>: > > > On Thu, May 17, 2018 at 07:03:29PM +0200, Guido Kiener wrote: > > > Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the > > > control pipe. Used by specific applications of IVI Foundation, > > > Inc. to implement VISA API functions: viUsbControlIn/Out. > > > > > > Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com> > > > Reviewed-by: Steve Bayless <steve_bayless@keysight.com> > > > --- > > > drivers/usb/class/usbtmc.c | 61 ++++++++++++++++++++++++++++++++++++ > > > include/uapi/linux/usb/tmc.h | 15 +++++++++ > > > 2 files changed, 76 insertions(+) > > > > > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > > > index 152e2daa9644..00c2e51a23a7 100644 > > > --- a/drivers/usb/class/usbtmc.c > > > +++ b/drivers/usb/class/usbtmc.c > > > @@ -5,6 +5,7 @@ > > > * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany > > > * Copyright (C) 2008 Novell, Inc. > > > * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> > > > + * Copyright (C) 2018, IVI Foundation, Inc. > > > */ > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > @@ -1263,6 +1264,62 @@ static int > > > usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) > > > return rv; > > > } > > > > > > +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, > > > + void __user *arg) > > > +{ > > > + struct device *dev = &data->intf->dev; > > > + struct usbtmc_ctrlrequest request; > > > + u8 *buffer = NULL; > > > + int rv; > > > + unsigned long res; > > > + > > > + res = copy_from_user(&request, arg, sizeof(struct usbtmc_ctrlrequest)); > > > + if (res) > > > + return -EFAULT; > > > + > > > + buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL); > > > > No validation that wLength is a sane number? > > Ooops. It should be buffer = kmalloc(max_t(u16, 256, request.req.wLength), > GFP_KERNEL); > Then all values of request.req.wLength (0..65535) are ok. Yes, that would make more sense. But you should still reject known-invalid values, and not just "silently fix them up". > > Go to the whiteboard nearby and write a the top of it: > > ALL INPUT IS EVIL > > > > > + if (!buffer) > > > + return -ENOMEM; > > > + > > > + if ((request.req.bRequestType & USB_DIR_IN) == 0 > > > + && request.req.wLength) { > > > + res = copy_from_user(buffer, request.data, > > > + request.req.wLength); > > > + if (res) { > > > + rv = -EFAULT; > > > + goto exit; > > > + } > > > + } > > > + > > > + rv = usb_control_msg(data->usb_dev, > > > + usb_rcvctrlpipe(data->usb_dev, 0), > > > + request.req.bRequest, > > > + request.req.bRequestType, > > > + request.req.wValue, > > > + request.req.wIndex, > > > + buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT); > > > > request.req.wLength might not be the actual size of buffer here due to > > your above min_t() check. Still wrong given the check above. > > Then there is the generic question of "why are you allowing arbitrary > > USB control messages to be sent to devices" here. That feels like a > > very odd way for a device that is supposed to be following a standard to > > be working. You are circumventing the standard here by allowing any > > message to be sent to the device. While nice for that one type of > > device, you are breaking interoperability in horrible ways (now apps > > only work with one type of device.) > > > > Why did you all agree to allow this to happen? > > I did not define the USBTMC and VISA standard. However the API requires > to send this generic control request. Otherwise we have to use the libusb > again. > > http://zone.ni.com/reference/en-XX/help/370131S-01/ni-visa/viusbcontrolin/ Ugh, that's horrid. And a sign of "who knows what our devices will want to support!" design by committee (note, I've worked on lots of spec groups before, I know how they work...) Ok, so that needs to be supported, fair enough, so let's get the implementation correct :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Zitat von Greg KH <gregkh@linuxfoundation.org>: > On Fri, May 18, 2018 at 01:32:51PM +0000, guido@kiener-muenchen.de wrote: >> >> Zitat von Greg KH <gregkh@linuxfoundation.org>: >> >> > On Thu, May 17, 2018 at 07:03:29PM +0200, Guido Kiener wrote: >> > > Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the >> > > control pipe. Used by specific applications of IVI Foundation, >> > > Inc. to implement VISA API functions: viUsbControlIn/Out. >> > > >> > > Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com> >> > > Reviewed-by: Steve Bayless <steve_bayless@keysight.com> >> > > --- >> > > drivers/usb/class/usbtmc.c | 61 ++++++++++++++++++++++++++++++++++++ >> > > include/uapi/linux/usb/tmc.h | 15 +++++++++ >> > > 2 files changed, 76 insertions(+) >> > > >> > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c >> > > index 152e2daa9644..00c2e51a23a7 100644 >> > > --- a/drivers/usb/class/usbtmc.c >> > > +++ b/drivers/usb/class/usbtmc.c >> > > @@ -5,6 +5,7 @@ >> > > * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany >> > > * Copyright (C) 2008 Novell, Inc. >> > > * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> >> > > + * Copyright (C) 2018, IVI Foundation, Inc. >> > > */ >> > > >> > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> > > @@ -1263,6 +1264,62 @@ static int >> > > usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) >> > > return rv; >> > > } >> > > >> > > +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, >> > > + void __user *arg) >> > > +{ >> > > + struct device *dev = &data->intf->dev; >> > > + struct usbtmc_ctrlrequest request; >> > > + u8 *buffer = NULL; >> > > + int rv; >> > > + unsigned long res; >> > > + >> > > + res = copy_from_user(&request, arg, sizeof(struct >> usbtmc_ctrlrequest)); >> > > + if (res) >> > > + return -EFAULT; >> > > + >> > > + buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL); >> > >> > No validation that wLength is a sane number? >> >> Ooops. It should be buffer = kmalloc(max_t(u16, 256, request.req.wLength), >> GFP_KERNEL); >> Then all values of request.req.wLength (0..65535) are ok. > > Yes, that would make more sense. But you should still reject > known-invalid values, and not just "silently fix them up". When I start here to refuse (current) unknown settings or flags, then I fear that people always want us to change or allow new flags. A device should always be able to deal with all possible values. We cannot prevent a user from sending crazy messages to the device and I do not want to develop something like a firewall here. > >> > Go to the whiteboard nearby and write a the top of it: >> > ALL INPUT IS EVIL >> > >> > > + if (!buffer) >> > > + return -ENOMEM; >> > > + >> > > + if ((request.req.bRequestType & USB_DIR_IN) == 0 >> > > + && request.req.wLength) { >> > > + res = copy_from_user(buffer, request.data, >> > > + request.req.wLength); >> > > + if (res) { >> > > + rv = -EFAULT; >> > > + goto exit; >> > > + } >> > > + } >> > > + >> > > + rv = usb_control_msg(data->usb_dev, >> > > + usb_rcvctrlpipe(data->usb_dev, 0), >> > > + request.req.bRequest, >> > > + request.req.bRequestType, >> > > + request.req.wValue, >> > > + request.req.wIndex, >> > > + buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT); >> > >> > request.req.wLength might not be the actual size of buffer here due to >> > your above min_t() check. > > Still wrong given the check above. I'm missing something. I do not see the error. The size of buffer is just at least 256 bytes. It doesn't matter when request.req.wLength < 256. I thought this helps the memory management, since we need not to deal with different and odd memory sizes. Maybe we should just alloc always a size of request.req.wLength. > >> > Then there is the generic question of "why are you allowing arbitrary >> > USB control messages to be sent to devices" here. That feels like a >> > very odd way for a device that is supposed to be following a standard to >> > be working. You are circumventing the standard here by allowing any >> > message to be sent to the device. While nice for that one type of >> > device, you are breaking interoperability in horrible ways (now apps >> > only work with one type of device.) >> > >> > Why did you all agree to allow this to happen? >> >> I did not define the USBTMC and VISA standard. However the API requires >> to send this generic control request. Otherwise we have to use the libusb >> again. >> >> http://zone.ni.com/reference/en-XX/help/370131S-01/ni-visa/viusbcontrolin/ > > Ugh, that's horrid. And a sign of "who knows what our devices will want > to support!" design by committee (note, I've worked on lots of spec > groups before, I know how they work...) > > Ok, so that needs to be supported, fair enough, so let's get the > implementation correct :) > > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 21, 2018 at 10:06:57PM +0000, guido@kiener-muenchen.de wrote: > > Zitat von Greg KH <gregkh@linuxfoundation.org>: > > > On Fri, May 18, 2018 at 01:32:51PM +0000, guido@kiener-muenchen.de wrote: > > > > > > Zitat von Greg KH <gregkh@linuxfoundation.org>: > > > > > > > On Thu, May 17, 2018 at 07:03:29PM +0200, Guido Kiener wrote: > > > > > Add USBTMC_IOCTL_CTRL_REQUEST to send arbitrary requests on the > > > > > control pipe. Used by specific applications of IVI Foundation, > > > > > Inc. to implement VISA API functions: viUsbControlIn/Out. > > > > > > > > > > Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com> > > > > > Reviewed-by: Steve Bayless <steve_bayless@keysight.com> > > > > > --- > > > > > drivers/usb/class/usbtmc.c | 61 ++++++++++++++++++++++++++++++++++++ > > > > > include/uapi/linux/usb/tmc.h | 15 +++++++++ > > > > > 2 files changed, 76 insertions(+) > > > > > > > > > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > > > > > index 152e2daa9644..00c2e51a23a7 100644 > > > > > --- a/drivers/usb/class/usbtmc.c > > > > > +++ b/drivers/usb/class/usbtmc.c > > > > > @@ -5,6 +5,7 @@ > > > > > * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany > > > > > * Copyright (C) 2008 Novell, Inc. > > > > > * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> > > > > > + * Copyright (C) 2018, IVI Foundation, Inc. > > > > > */ > > > > > > > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > > @@ -1263,6 +1264,62 @@ static int > > > > > usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) > > > > > return rv; > > > > > } > > > > > > > > > > +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, > > > > > + void __user *arg) > > > > > +{ > > > > > + struct device *dev = &data->intf->dev; > > > > > + struct usbtmc_ctrlrequest request; > > > > > + u8 *buffer = NULL; > > > > > + int rv; > > > > > + unsigned long res; > > > > > + > > > > > + res = copy_from_user(&request, arg, sizeof(struct > > > usbtmc_ctrlrequest)); > > > > > + if (res) > > > > > + return -EFAULT; > > > > > + > > > > > + buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL); > > > > > > > > No validation that wLength is a sane number? > > > > > > Ooops. It should be buffer = kmalloc(max_t(u16, 256, request.req.wLength), > > > GFP_KERNEL); > > > Then all values of request.req.wLength (0..65535) are ok. > > > > Yes, that would make more sense. But you should still reject > > known-invalid values, and not just "silently fix them up". > > When I start here to refuse (current) unknown settings or flags, then I fear > that people always want us to change or allow new flags. That doesn't make sense, you always need to reject unknown values, or people will put crap in them and then get mad in the future when you want to use those fields/values for something else. > A device should always be able to deal with all possible values. But the kernel has to be sane and properly validate userspace values. > We > cannot prevent a > user from sending crazy messages to the device and I do not want to develop > something like a firewall here. Sure, I'm not saying to do that, you are not validating the other fields, just the length. The kernel will validate the other fields when you submit the urb. > > > > > + rv = usb_control_msg(data->usb_dev, > > > > > + usb_rcvctrlpipe(data->usb_dev, 0), > > > > > + request.req.bRequest, > > > > > + request.req.bRequestType, > > > > > + request.req.wValue, > > > > > + request.req.wIndex, > > > > > + buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT); > > > > > > > > request.req.wLength might not be the actual size of buffer here due to > > > > your above min_t() check. > > > > Still wrong given the check above. > > I'm missing something. I do not see the error. The size of buffer is just > at least 256 bytes. It doesn't matter when request.req.wLength < 256. If wLength is set to 10000 but buffer really isn't that big, bad things happen... > I thought this helps the memory management, since we need not to deal with > different and odd memory sizes. Maybe we should just alloc always a size of > request.req.wLength. Again, you have to validate that number, if you want to bound it to 256, great, then bound it. Don't half-way do it please. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c index 152e2daa9644..00c2e51a23a7 100644 --- a/drivers/usb/class/usbtmc.c +++ b/drivers/usb/class/usbtmc.c @@ -5,6 +5,7 @@ * Copyright (C) 2007 Stefan Kopp, Gechingen, Germany * Copyright (C) 2008 Novell, Inc. * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> + * Copyright (C) 2018, IVI Foundation, Inc. */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt @@ -1263,6 +1264,62 @@ static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data) return rv; } +static int usbtmc_ioctl_request(struct usbtmc_device_data *data, + void __user *arg) +{ + struct device *dev = &data->intf->dev; + struct usbtmc_ctrlrequest request; + u8 *buffer = NULL; + int rv; + unsigned long res; + + res = copy_from_user(&request, arg, sizeof(struct usbtmc_ctrlrequest)); + if (res) + return -EFAULT; + + buffer = kmalloc(min_t(u16, 256, request.req.wLength), GFP_KERNEL); + if (!buffer) + return -ENOMEM; + + if ((request.req.bRequestType & USB_DIR_IN) == 0 + && request.req.wLength) { + res = copy_from_user(buffer, request.data, + request.req.wLength); + if (res) { + rv = -EFAULT; + goto exit; + } + } + + rv = usb_control_msg(data->usb_dev, + usb_rcvctrlpipe(data->usb_dev, 0), + request.req.bRequest, + request.req.bRequestType, + request.req.wValue, + request.req.wIndex, + buffer, request.req.wLength, USB_CTRL_GET_TIMEOUT); + + if (rv < 0) { + dev_err(dev, "%s failed %d\n", __func__, rv); + goto exit; + } + if ((request.req.bRequestType & USB_DIR_IN)) { + if (rv > request.req.wLength) { + dev_warn(dev, "%s returned too much data: %d\n", + __func__, rv); + rv = request.req.wLength; + } + + res = copy_to_user(request.data, buffer, rv); + if (res) + rv = -EFAULT; + } + + exit: + kfree(buffer); + return rv; +} + /* * Get the usb timeout value */ @@ -1379,6 +1436,10 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg) retval = usbtmc_ioctl_abort_bulk_in(data); break; + case USBTMC_IOCTL_CTRL_REQUEST: + retval = usbtmc_ioctl_request(data, (void __user *)arg); + break; + case USBTMC_IOCTL_GET_TIMEOUT: retval = usbtmc_ioctl_get_timeout(file_data, (void __user *)arg); diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h index 60369825691c..c1acec9ad834 100644 --- a/include/uapi/linux/usb/tmc.h +++ b/include/uapi/linux/usb/tmc.h @@ -4,6 +4,7 @@ * Copyright (C) 2008 Novell, Inc. * Copyright (C) 2008 Greg Kroah-Hartman <gregkh@suse.de> * Copyright (C) 2015 Dave Penkler <dpenkler@gmail.com> + * Copyright (C) 2018, IVI Foundation, Inc. * * This file holds USB constants defined by the USB Device Class * and USB488 Subclass Definitions for Test and Measurement devices @@ -40,6 +41,19 @@ #define USBTMC488_REQUEST_GOTO_LOCAL 161 #define USBTMC488_REQUEST_LOCAL_LOCKOUT 162 +struct usbtmc_request { + __u8 bRequestType; + __u8 bRequest; + __u16 wValue; + __u16 wIndex; + __u16 wLength; +} __attribute__ ((packed)); + +struct usbtmc_ctrlrequest { + struct usbtmc_request req; + void __user *data; +} __attribute__ ((packed)); + struct usbtmc_termchar { __u8 term_char; __u8 term_char_enabled; // bool @@ -53,6 +67,7 @@ struct usbtmc_termchar { #define USBTMC_IOCTL_ABORT_BULK_IN _IO(USBTMC_IOC_NR, 4) #define USBTMC_IOCTL_CLEAR_OUT_HALT _IO(USBTMC_IOC_NR, 6) #define USBTMC_IOCTL_CLEAR_IN_HALT _IO(USBTMC_IOC_NR, 7) +#define USBTMC_IOCTL_CTRL_REQUEST _IOWR(USBTMC_IOC_NR, 8, struct usbtmc_ctrlrequest) #define USBTMC_IOCTL_GET_TIMEOUT _IOR(USBTMC_IOC_NR, 9, unsigned int) #define USBTMC_IOCTL_SET_TIMEOUT _IOW(USBTMC_IOC_NR, 10, unsigned int) #define USBTMC_IOCTL_EOM_ENABLE _IOW(USBTMC_IOC_NR, 11, __u8)