Fix erroneous ioctl returns
Subsystems (2)
v2 cover Fix erroneous ioctl returns
Recent tests with timeouts > INT_MAX produced random error returns with usbtmc_get_stb. This was caused by assigning the return value of wait_event_interruptible_timeout to an int which overflowed to negative values. Also return value on success was the remaining number of jiffies instead of 0.
These patches fix all the cases where the return of wait_event_interruptible_timeout was assigned to an int and the case of the remaining jiffies return in usbtmc_get_stb.
Patch 1: Fixes usbtmc_get_stb
Patch 2: Fixes usbtmc488_ioctl_wait_srq
Patch 3: Fixes usbtmc_generic_read
Dave Penkler (3):
usb: usbtmc: Fix erroneous get_stb ioctl error returns
usb: usbtmc: Fix erroneous wait_srq ioctl return
usb: usbtmc: Fix erroneous generic_read ioctl return
drivers/usb/class/usbtmc.c | 53 ++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 22 deletions(-)
--
Changes V1 => V2 Add cc to stable line
2.49.0
PATCH v3 1/3 usb: usbtmc: Fix erroneous get_stb ioctl error returns
wait_event_interruptible_timeout returns a long
The return was being assigned to an int causing an integer overflow when the remaining jiffies > INT_MAX resulting in random error returns.
Use a long return value and convert to int ioctl return only on error.
When the return value of wait_event_interruptible_timeout was <= INT_MAX the number of remaining jiffies was returned which has no meaning for the user. Return 0 on success.
---
Change V1 -> V2
Correct commit message wrongly stating the return value on success was from usb_control_msg
Change V2 -> V3
Add cc to stable line
drivers/usb/class/usbtmc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/usb/class/usbtmc.c +8 -4
@@ -482,6 +482,7 @@ static int usbtmc_get_stb(struct usbtmc_file_data *file_data, __u8 *stb)u8 *buffer;u8 tag;int rv; +long wait_rv;dev_dbg (dev,"Enter ioctl_read_stb iin_ep_present: %d\n" , data->iin_ep_present);
@@ -511,16 +512,17 @@ static int usbtmc_get_stb(struct usbtmc_file_data *file_data, __u8 *stb) }if (data->iin_ep_present) { - rv =wait_event_interruptible_timeout ( + wait_rv =wait_event_interruptible_timeout ( data->waitq,atomic_read (&data->iin_data_valid) !=0 , file_data->timeout); -if (rv <0 ) { -dev_dbg (dev,"wait interrupted %d\n" , rv); +if (wait_rv <0 ) { +dev_dbg (dev,"wait interrupted %ld\n" , wait_rv); + rv = wait_rv; goto exit; } -if (rv ==0 ) { +if (wait_rv ==0 ) {dev_dbg (dev,"wait timed out\n" ); rv = -ETIMEDOUT ; goto exit;
@@ -539,6 +541,8 @@ static int usbtmc_get_stb(struct usbtmc_file_data *file_data, __u8 *stb)dev_dbg (dev,"stb:0x%02x received %d\n" , (unsigned int )*stb, rv); + rv =0 ; + exit:/* bump interrupt bTag */ data->iin_bTag +=1 ;
Signature · 1 line
2.49.0
PATCH v2 2/3 usb: usbtmc: Fix erroneous wait_srq ioctl return
In reply to Dave Penklerwait_event_interruptible_timeout returns a long
The return was being assigned to an int causing an integer overflow when the remaining jiffies > INT_MAX resulting in random error returns.
Use a long return value, converting to the int ioctl return only on error.
---
Change V1 -> V2
Add cc to stable line
drivers/usb/class/usbtmc.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
drivers/usb/class/usbtmc.c +11 -12
@@ -606,9 +606,9 @@ static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data, {struct usbtmc_device_data *data = file_data->data;struct device *dev = &data->intf->dev; -int rv;u32 timeout;unsigned long expire; +long wait_rv;if (!data->iin_ep_present) {dev_dbg (dev,"no interrupt endpoint present\n" );
@@ -622,25 +622,24 @@ static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data,mutex_unlock (&data->io_mutex); - rv =wait_event_interruptible_timeout ( - data->waitq, -atomic_read (&file_data->srq_asserted) !=0 || -atomic_read (&file_data->closing), - expire); + wait_rv =wait_event_interruptible_timeout ( + data->waitq, +atomic_read (&file_data->srq_asserted) !=0 || +atomic_read (&file_data->closing), + expire);mutex_lock (&data->io_mutex);/* Note! disconnect or close could be called in the meantime */ if (atomic_read (&file_data->closing) || data->zombie) - rv = -ENODEV ; +return -ENODEV ; -if (rv <0 ) { -/* dev can be invalid now! */ -pr_debug ("%s - wait interrupted %d\n" , __func__, rv); -return rv; +if (wait_rv <0 ) { +dev_dbg (dev,"%s - wait interrupted %ld\n" , __func__, wait_rv); +return wait_rv; } -if (rv ==0 ) { +if (wait_rv ==0 ) {dev_dbg (dev,"%s - wait timed out\n" , __func__);return -ETIMEDOUT ; }
Signature · 1 line
2.49.0
PATCH v2 3/3 usb: usbtmc: Fix erroneous generic_read ioctl return
In reply to Dave Penklerwait_event_interruptible_timeout returns a long
The return value was being assigned to an int causing an integer overflow when the remaining jiffies > INT_MAX which resulted in random error returns.
Use a long return value, converting to the int ioctl return only on error.
---
Change V1 -> V2
Acc cc to stable line
drivers/usb/class/usbtmc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
drivers/usb/class/usbtmc.c +12 -6
@@ -833,6 +833,7 @@ static ssize_t usbtmc_generic_read(struct usbtmc_file_data *file_data,unsigned long expire;int bufcount =1 ;int again =0 ; +long wait_rv;/* mutex already locked */
@@ -945,19 +946,24 @@ static ssize_t usbtmc_generic_read(struct usbtmc_file_data *file_data,if (!(flags &USBTMC_FLAG_ASYNC )) {dev_dbg (dev,"%s: before wait time %lu\n" , __func__, expire); - retval =wait_event_interruptible_timeout ( + wait_rv =wait_event_interruptible_timeout ( file_data->wait_bulk_in,usbtmc_do_transfer (file_data), expire); -dev_dbg (dev,"%s: wait returned %d\n" , - __func__, retval); +dev_dbg (dev,"%s: wait returned %ld\n" , + __func__, wait_rv); + +if (wait_rv <0 ) { + retval = wait_rv; + goto error; + } -if (retval <=0 ) { -if (retval ==0 ) - retval = -ETIMEDOUT ; +if (wait_rv ==0 ) { + retval = -ETIMEDOUT ; goto error; } + } urb =usb_get_from_anchor (&file_data->in_anchor);
Signature · 1 line
2.49.0