4.14.203-rc1 review
Subsystems (2)
Greg Kroah-Hartman
maintainerPATCH 183/191 usb: core: Solve race condition in anchor cleanup functions
In reply to Greg Kroah-HartmanFrom: Eli Billauer <[email protected]>
[ Upstream commit fbc299437c06648afcc7891e6e2e6638dd48d4df ]
usb_kill_anchored_urbs() is commonly used to cancel all URBs on an anchor just before releasing resources which the URBs rely on. By doing so, users of this function rely on that no completer callbacks will take place from any URB on the anchor after it returns.
However if this function is called in parallel with __usb_hcd_giveback_urb processing a URB on the anchor, the latter may call the completer callback after usb_kill_anchored_urbs() returns. This can lead to a kernel panic due to use after release of memory in interrupt context.
The race condition is that __usb_hcd_giveback_urb() first unanchors the URB and then makes the completer callback. Such URB is hence invisible to usb_kill_anchored_urbs(), allowing it to return before the completer has been called, since the anchor's urb_list is empty.
Even worse, if the racing completer callback resubmits the URB, it may remain in the system long after usb_kill_anchored_urbs() returns.
Hence list_empty(&anchor->urb_list), which is used in the existing while-loop, doesn't reliably ensure that all URBs of the anchor are gone.
A similar problem exists with usb_poison_anchored_urbs() and usb_scuttle_anchored_urbs().
This patch adds an external do-while loop, which ensures that all URBs are indeed handled before these three functions return. This change has no effect at all unless the race condition occurs, in which case the loop will busy-wait until the racing completer callback has finished. This is a rare condition, so the CPU waste of this spinning is negligible.
The additional do-while loop relies on usb_anchor_check_wakeup(), which returns true iff the anchor list is empty, and there is no __usb_hcd_giveback_urb() in the system that is in the middle of the unanchor-before-complete phase. The @suspend_wakeups member of struct usb_anchor is used for this purpose, which was introduced to solve another problem which the same race condition causes, in commit 6ec4147e7bdb ("usb-anchor: Delay usb_wait_anchor_empty_timeout wake up till completion is done").
The surely_empty variable is necessary, because usb_anchor_check_wakeup() must be called with the lock held to prevent races. However the spinlock must be released and reacquired if the outer loop spins with an empty URB list while waiting for the unanchor-before-complete passage to finish:
The completer callback may very well attempt to take the very same lock.
To summarize, using usb_anchor_check_wakeup() means that the patched functions can return only when the anchor's list is empty, and there is no invisible URB being processed. Since the inner while loop finishes on the empty list condition, the new do-while loop will terminate as well, except for when the said race condition occurs.
--- drivers/usb/core/urb.c | 89 +++++++++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 35 deletions(-)
drivers/usb/core/urb.c +54 -35
@@ -767,11 +767,12 @@ void usb_block_urb(struct urb *urb)EXPORT_SYMBOL_GPL (usb_block_urb);/** -* usb_kill_anchored_urbs - cancel transfer requests en masse +* usb_kill_anchored_urbs - kill all URBs associated with an anchor * @anchor: anchor the requests are bound to * -* this allows all outstanding URBs to be killed starting -* from the back of the queue +* This kills all outstanding URBs starting from the back of the queue, +* with guarantee that no completer callbacks will take place from the +* anchor after this function returns. * * This routine should not be called by a driver after its disconnect * method has returned.
@@ -779,20 +780,26 @@ EXPORT_SYMBOL_GPL(usb_block_urb);void usb_kill_anchored_urbs (struct usb_anchor *anchor) {struct urb *victim; +int surely_empty; -spin_lock_irq (&anchor->lock); -while (!list_empty (&anchor->urb_list)) { - victim = list_entry(anchor->urb_list.prev, struct urb, - anchor_list); -/* we must make sure the URB isn't freed before we kill it*/ -usb_get_urb (victim); -spin_unlock_irq (&anchor->lock); -/* this will unanchor the URB */ -usb_kill_urb (victim); -usb_put_urb (victim); +do {spin_lock_irq (&anchor->lock); - } -spin_unlock_irq (&anchor->lock); +while (!list_empty (&anchor->urb_list)) { + victim =list_entry (anchor->urb_list.prev, + struct urb, anchor_list); +/* make sure the URB isn't freed before we kill it */ +usb_get_urb (victim); +spin_unlock_irq (&anchor->lock); +/* this will unanchor the URB */ +usb_kill_urb (victim); +usb_put_urb (victim); +spin_lock_irq (&anchor->lock); + } + surely_empty =usb_anchor_check_wakeup (anchor); + +spin_unlock_irq (&anchor->lock); +cpu_relax (); + }while (!surely_empty); }EXPORT_SYMBOL_GPL (usb_kill_anchored_urbs);
@@ -811,21 +818,27 @@ EXPORT_SYMBOL_GPL(usb_kill_anchored_urbs);void usb_poison_anchored_urbs (struct usb_anchor *anchor) {struct urb *victim; +int surely_empty; -spin_lock_irq (&anchor->lock); - anchor->poisoned =1 ; -while (!list_empty (&anchor->urb_list)) { - victim = list_entry(anchor->urb_list.prev, struct urb, - anchor_list); -/* we must make sure the URB isn't freed before we kill it*/ -usb_get_urb (victim); -spin_unlock_irq (&anchor->lock); -/* this will unanchor the URB */ -usb_poison_urb (victim); -usb_put_urb (victim); +do {spin_lock_irq (&anchor->lock); - } -spin_unlock_irq (&anchor->lock); + anchor->poisoned =1 ; +while (!list_empty (&anchor->urb_list)) { + victim =list_entry (anchor->urb_list.prev, + struct urb, anchor_list); +/* make sure the URB isn't freed before we kill it */ +usb_get_urb (victim); +spin_unlock_irq (&anchor->lock); +/* this will unanchor the URB */ +usb_poison_urb (victim); +usb_put_urb (victim); +spin_lock_irq (&anchor->lock); + } + surely_empty =usb_anchor_check_wakeup (anchor); + +spin_unlock_irq (&anchor->lock); +cpu_relax (); + }while (!surely_empty); }EXPORT_SYMBOL_GPL (usb_poison_anchored_urbs);
@@ -965,14 +978,20 @@ void usb_scuttle_anchored_urbs(struct usb_anchor *anchor) {struct urb *victim;unsigned long flags; +int surely_empty; + +do { +spin_lock_irqsave (&anchor->lock, flags); +while (!list_empty (&anchor->urb_list)) { + victim =list_entry (anchor->urb_list.prev, + struct urb, anchor_list); +__usb_unanchor_urb (victim, anchor); + } + surely_empty =usb_anchor_check_wakeup (anchor); -spin_lock_irqsave (&anchor->lock, flags); -while (!list_empty (&anchor->urb_list)) { - victim = list_entry(anchor->urb_list.prev, struct urb, - anchor_list); -__usb_unanchor_urb (victim, anchor); - } -spin_unlock_irqrestore (&anchor->lock, flags); +spin_unlock_irqrestore (&anchor->lock, flags); +cpu_relax (); + }while (!surely_empty); }EXPORT_SYMBOL_GPL (usb_scuttle_anchored_urbs);
Signature · 1 line
2.25.1
Greg Kroah-Hartman
maintainerPATCH 184/191 scsi: ufs: ufs-qcom: Fix race conditions caused by ufs_qcom_testbus_config()
In reply to Greg Kroah-HartmanFrom: Can Guo <[email protected]>
[ Upstream commit 89dd87acd40a44de8ff3358138aedf8f73f4efc6 ]
If ufs_qcom_dump_dbg_regs() calls ufs_qcom_testbus_config() from ufshcd_suspend/resume and/or clk gate/ungate context, pm_runtime_get_sync() and ufshcd_hold() will cause a race condition. Fix this by removing the unnecessary calls of pm_runtime_get_sync() and ufshcd_hold().
--- drivers/scsi/ufs/ufs-qcom.c | 5 ----- 1 file changed, 5 deletions(-)
drivers/scsi/ufs/ufs-qcom.c +0 -5
@@ -1595,9 +1595,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)*/ } mask <<= offset; - -pm_runtime_get_sync (host->hba->dev); -ufshcd_hold (host->hba, false);ufshcd_rmwl (host->hba,TEST_BUS_SEL , (u32 )host->testbus.select_major <<19 ,REG_UFS_CFG1 );
@@ -1610,8 +1607,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)* committed before returning. */ mb (); -ufshcd_release (host->hba); -pm_runtime_put_sync (host->hba->dev);return 0 ; }
Signature · 1 line
2.25.1
Greg Kroah-Hartman
maintainerPATCH 185/191 ath10k: check idx validity in __ath10k_htt_rx_ring_fill_n()
In reply to Greg Kroah-HartmanFrom: Zekun Shen <[email protected]>
[ Upstream commit bad60b8d1a7194df38fd7fe4b22f3f4dcf775099 ]
The idx in __ath10k_htt_rx_ring_fill_n function lives in consistent dma region writable by the device. Malfunctional or malicious device could manipulate such idx to have a OOB write. Either by
htt->rx_ring.netbufs_ring[idx] = skb;
or by
ath10k_htt_set_paddrs_ring(htt, paddr, idx);The idx can also be negative as it's signed, giving a large memory space to write to.
It's possibly exploitable by corruptting a legit pointer with a skb pointer. And then fill skb with payload as rougue object.
Part of the log here. Sometimes it appears as UAF when writing to a freed memory by chance.
[ 15.594376] BUG: unable to handle page fault for address: ffff887f5c1804f0 [ 15.595483] #PF: supervisor write access in kernel mode [ 15.596250] #PF: error_code(0x0002) - not-present page [ 15.597013] PGD 0 P4D 0 [ 15.597395] Oops: 0002 [#1] SMP KASAN PTI [ 15.597967] CPU: 0 PID: 82 Comm: kworker/u2:2 Not tainted 5.6.0 #69 [ 15.598843] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 [ 15.600438] Workqueue: ath10k_wq ath10k_core_register_work [ath10k_core] [ 15.601389] RIP: 0010:__ath10k_htt_rx_ring_fill_n (linux/drivers/net/wireless/ath/ath10k/htt_rx.c:173) ath10k_core
--- drivers/net/wireless/ath/ath10k/htt_rx.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/net/wireless/ath/ath10k/htt_rx.c +8 -0
@@ -100,6 +100,14 @@ static int __ath10k_htt_rx_ring_fill_n(struct ath10k_htt *htt, int num)BUILD_BUG_ON (HTT_RX_RING_FILL_LEVEL >=HTT_RX_RING_SIZE /2 ); idx =__le32_to_cpu (*htt->rx_ring.alloc_idx.vaddr); + +if (idx <0 || idx >= htt->rx_ring.size) { +ath10k_err (htt->ar,"rx ring index is not valid, firmware malfunctioning?\n" ); + idx &= htt->rx_ring.size_mask; + ret = -ENOMEM ; + goto fail; + } +while (num >0 ) { skb =dev_alloc_skb (HTT_RX_BUF_SIZE +HTT_RX_DESC_ALIGN );if (!skb) {
Signature · 1 line
2.25.1
Greg Kroah-Hartman
maintainerPATCH 186/191 net: korina: cast KSEG0 address to pointer in kfree
In reply to Greg Kroah-HartmanFrom: Valentin Vidic <[email protected]>
[ Upstream commit 3bd57b90554b4bb82dce638e0668ef9dc95d3e96 ]
Fixes gcc warning:
passing argument 1 of 'kfree' makes pointer from integer without a cast
--- drivers/net/ethernet/korina.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/korina.c +2 -2
@@ -1188,7 +1188,7 @@ static int korina_probe(struct platform_device *pdev)return rc; probe_err_register: -kfree (KSEG0ADDR (lp->td_ring)); +kfree ((struct dma_desc *)KSEG0ADDR (lp->td_ring)); probe_err_td_ring:iounmap (lp->tx_dma_regs); probe_err_dma_tx:
@@ -1208,7 +1208,7 @@ static int korina_remove(struct platform_device *pdev)iounmap (lp->eth_regs);iounmap (lp->rx_dma_regs);iounmap (lp->tx_dma_regs); -kfree (KSEG0ADDR (lp->td_ring)); +kfree ((struct dma_desc *)KSEG0ADDR (lp->td_ring));unregister_netdev (bif->dev);free_netdev (bif->dev);
Signature · 1 line
2.25.1
Greg Kroah-Hartman
maintainerPATCH 187/191 tty: serial: fsl_lpuart: fix lpuart32_poll_get_char
In reply to Greg Kroah-HartmanFrom: Peng Fan <[email protected]>
commit 29788ab1d2bf26c130de8f44f9553ee78a27e8d5 upstream.
The watermark is set to 1, so we need to input two chars to trigger RDRF using the original logic. With the new logic, we could always get the char when there is data in FIFO.
--- drivers/tty/serial/fsl_lpuart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/fsl_lpuart.c +1 -1
@@ -567,7 +567,7 @@ static void lpuart32_poll_put_char(strucstatic int lpuart32_poll_get_char (struct uart_port *port) { -if (!(lpuart32_read (port,UARTSTAT ) &UARTSTAT_RDRF )) +if (!(lpuart32_read (port,UARTWATER ) >>UARTWATER_RXCNT_OFF ))return NO_POLL_CHAR ;return lpuart32_read (port,UARTDATA );
Greg Kroah-Hartman
maintainerPATCH 188/191 usb: cdc-acm: add quirk to blacklist ETAS ES58X devices
In reply to Greg Kroah-HartmanFrom: Vincent Mailhol <[email protected]>
commit a4f88430af896bf34ec25a7a5f0e053fb3d928e0 upstream.
The ES58X devices has a CDC ACM interface (used for debug
purpose). During probing, the device is thus recognized as USB Modem (CDC ACM), preventing the etas-es58x module to load:
usbcore: registered new interface driver etas_es58x
usb 1-1.1: new full-speed USB device number 14 using xhci_hcd
usb 1-1.1: New USB device found, idVendor=108c, idProduct=0159, bcdDevice= 1.00 usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3 usb 1-1.1: Product: ES581.4
usb 1-1.1: Manufacturer: ETAS GmbH
usb 1-1.1: SerialNumber: 2204355
cdc_acm 1-1.1:1.0: No union descriptor, testing for castrated device cdc_acm 1-1.1:1.0: ttyACM0: USB ACM device
Thus, these have been added to the ignore list indrivers/usb/class/cdc-acm.c
N.B. Future firmware release of the ES58X will remove the CDC-ACM interface.
lsusb -v of the three devices variant (ES581.4, ES582.1 and ES584.1):
Bus 001 Device 011: ID 108c:0159 Robert Bosch GmbH ES581.4 Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x108c Robert Bosch GmbH
idProduct 0x0159
bcdDevice 1.00
iManufacturer 1 ETAS GmbH
iProduct 2 ES581.4
iSerial 3 2204355
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0035
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 5 Bus Powered Configuration
bmAttributes 0x80
(Bus Powered)
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 0
iInterface 4 ACM Control Interface
CDC Header:
bcdCDC 1.10
CDC Call Management:
bmCapabilities 0x01
call management
bDataInterface 0
CDC ACM:
bmCapabilities 0x06
sends break
line coding and serial state
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 10
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Device Status: 0x0000
(Bus Powered)
Bus 001 Device 012: ID 108c:0168 Robert Bosch GmbH ES582 Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x108c Robert Bosch GmbH
idProduct 0x0168
bcdDevice 1.00
iManufacturer 1 ETAS GmbH
iProduct 2 ES582
iSerial 3 0108933
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0043
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 0
bmAttributes 0x80
(Bus Powered)
MaxPower 500mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 1 AT-commands (v.25ter)
iInterface 0
CDC Header:
bcdCDC 1.10
CDC ACM:
bmCapabilities 0x02
line coding and serial state
CDC Union:
bMasterInterface 0
bSlaveInterface 1
CDC Call Management:
bmCapabilities 0x03
call management
use DataInterface
bDataInterface 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 16
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 10 CDC Data
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Device Qualifier (for other device speed):
bLength 10
bDescriptorType 6
bcdUSB 2.00
bDeviceClass 2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
bNumConfigurations 1
Device Status: 0x0000
(Bus Powered)
Bus 001 Device 013: ID 108c:0169 Robert Bosch GmbH ES584.1 Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x108c Robert Bosch GmbH
idProduct 0x0169
bcdDevice 1.00
iManufacturer 1 ETAS GmbH
iProduct 2 ES584.1
iSerial 3 0100320
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 0x0043
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 0
bmAttributes 0x80
(Bus Powered)
MaxPower 500mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 1 AT-commands (v.25ter)
iInterface 0
CDC Header:
bcdCDC 1.10
CDC ACM:
bmCapabilities 0x02
line coding and serial state
CDC Union:
bMasterInterface 0
bSlaveInterface 1
CDC Call Management:
bmCapabilities 0x03
call management
use DataInterface
bDataInterface 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 16
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 10 CDC Data
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Device Qualifier (for other device speed):
bLength 10
bDescriptorType 6
bcdUSB 2.00
bDeviceClass 2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
bNumConfigurations 1
Device Status: 0x0000
(Bus Powered)
--- drivers/usb/class/cdc-acm.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/usb/class/cdc-acm.c +11 -0
@@ -1957,6 +1957,17 @@ static const struct usb_device_id acm_id .driver_info = IGNORE_DEVICE, }, +/* Exclude ETAS ES58x */ + { USB_DEVICE(0x108c, 0x0159), /* ES581.4 */ + .driver_info =IGNORE_DEVICE , + }, + { USB_DEVICE(0x108c, 0x0168), /* ES582.1 */ + .driver_info = IGNORE_DEVICE, + }, + { USB_DEVICE(0x108c, 0x0169), /* ES584.1 */ + .driver_info = IGNORE_DEVICE, + }, + { USB_DEVICE(0x1bc7, 0x0021), /* Telit 3G ACM only composition */ .driver_info = SEND_ZERO_PACKET, },
Greg Kroah-Hartman
maintainerPATCH 189/191 USB: cdc-wdm: Make wdm_flush() interruptible and add wdm_fsync().
In reply to Greg Kroah-HartmanFrom: Oliver Neukum <[email protected]>
commit 37d2a36394d954413a495da61da1b2a51ecd28ab upstream.
syzbot is reporting hung task at wdm_flush() [1], for there is a circular dependency that wdm_flush() from flip_close() for /dev/cdc-wdm0 forever waits for /dev/raw-gadget to be closed while close() for /dev/raw-gadget cannot be called unless close() for /dev/cdc-wdm0 completes.
Tetsuo Handa considered that such circular dependency is an usage error [2] which corresponds to an unresponding broken hardware [3]. But Alan Stern responded that we should be prepared for such hardware [4]. Therefore, this patch changes wdm_flush() to use wait_event_interruptible_timeout() which gives up after 30 seconds, for hardware that remains silent must be ignored. The 30 seconds are coming out of thin air.
Changing wait_event() to wait_event_interruptible_timeout() makes error reporting from close() syscall less reliable. To compensate it, this patch also implements wdm_fsync() which does not use timeout. Those who want to be very sure that data has gone out to the device are now advised to call fsync(), with a caveat that fsync() can return -EINVAL when running on older kernels which do not implement wdm_fsync().
This patch also fixes three more problems (listed below) found during exhaustive discussion and testing.
Since multiple threads can concurrently call wdm_write()/wdm_flush(), we need to use wake_up_all() whenever clearing WDM_IN_USE in order to make sure that all waiters are woken up. Also, error reporting needs to use fetch-and-clear approach in order not to report same error for multiple times.
Since wdm_flush() checks WDM_DISCONNECTING, wdm_write() should as well check WDM_DISCONNECTING.
In wdm_flush(), since locks are not held, it is not safe to dereference desc->intf after checking that WDM_DISCONNECTING is not set [5]. Thus, remove dev_err() from wdm_flush().
[1] https://syzkaller.appspot.com/bug?id=e7b761593b23eb50855b9ea31e3be5472b711186
[2] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
[4] https://lkml.kernel.org/r/[email protected]
[5] https://lkml.kernel.org/r/[email protected]
--- drivers/usb/class/cdc-wdm.c | 70 +++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 16 deletions(-)
drivers/usb/class/cdc-wdm.c +54 -16
@@ -57,6 +57,9 @@ MODULE_DEVICE_TABLE (usb, wdm_ids);#define WDM_MAX 16 +/* we cannot wait forever at flush() */ +#define WDM_FLUSH_TIMEOUT (30 * HZ) +/* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */ #define WDM_DEFAULT_BUFSIZE 256
@@ -147,7 +150,7 @@ static void wdm_out_callback(struct urbkfree (desc->outbuf); desc->outbuf =NULL ;clear_bit (WDM_IN_USE , &desc->flags); -wake_up (&desc->wait); +wake_up_all (&desc->wait); }/* forward declaration */
@@ -390,6 +393,9 @@ static ssize_t wdm_writeif (test_bit (WDM_RESETTING , &desc->flags)) r = -EIO ; +if (test_bit (WDM_DISCONNECTING , &desc->flags)) + r = -ENODEV ; +if (r <0 ) { rv = r; goto out_free_mem_pm;
@@ -421,6 +427,7 @@ static ssize_t wdm_writeif (rv <0 ) { desc->outbuf =NULL ;clear_bit (WDM_IN_USE , &desc->flags); +wake_up_all (&desc->wait);/* for wdm_wait_for_response() */ dev_err (&desc->intf->dev,"Tx URB error: %d\n" , rv); rv =usb_translate_errors (rv); goto out_free_mem_pm;
@@ -580,28 +587,58 @@ err:return rv; } -static int wdm_flush (struct file *file,fl_owner_t id) +static int wdm_wait_for_response (struct file *file,long timeout) {struct wdm_device *desc = file->private_data; +long rv;/* Use long here because (int) MAX_SCHEDULE_TIMEOUT < 0. */ -wait_event (desc->wait, -/* -* needs both flags. We cannot do with one -* because resetting it would cause a race -* with write() yet we need to signal -* a disconnect -*/ - !test_bit (WDM_IN_USE , &desc->flags) || -test_bit (WDM_DISCONNECTING , &desc->flags)); +/* +* Needs both flags. We cannot do with one because resetting it would +* cause a race with write() yet we need to signal a disconnect. +*/ + rv =wait_event_interruptible_timeout (desc->wait, + !test_bit (WDM_IN_USE , &desc->flags) || +test_bit (WDM_DISCONNECTING , &desc->flags), + timeout); -/* cannot dereference desc->intf if WDM_DISCONNECTING */ +/* +* To report the correct error. This is best effort. +* We are inevitably racing with the hardware. +*/ if (test_bit (WDM_DISCONNECTING , &desc->flags))return -ENODEV ; -if (desc->werr <0 ) -dev_err (&desc->intf->dev,"Error in flush path: %d\n" , - desc->werr); +if (!rv) +return -EIO ; +if (rv <0 ) +return -EINTR ; + +spin_lock_irq (&desc->iuspin); + rv = desc->werr; + desc->werr =0 ; +spin_unlock_irq (&desc->iuspin); + +return usb_translate_errors (rv); + +} -return usb_translate_errors (desc->werr); +/* +* You need to send a signal when you react to malicious or defective hardware. +* Also, don't abort when fsync() returned -EINVAL, for older kernels which do +* not implement wdm_flush() will return -EINVAL. +*/ +static int wdm_fsync (struct file *file,loff_t start,loff_t end,int datasync) +{ +return wdm_wait_for_response (file,MAX_SCHEDULE_TIMEOUT ); +} + +/* +* Same with wdm_fsync(), except it uses finite timeout in order to react to +* malicious or defective hardware which ceased communication after close() was +* implicitly called due to process termination. +*/ +static int wdm_flush (struct file *file,fl_owner_t id) +{ +return wdm_wait_for_response (file,WDM_FLUSH_TIMEOUT ); }static unsigned int wdm_poll (struct file *file,struct poll_table_struct *wait)
@@ -726,6 +763,7 @@ static const struct file_operations wdm_ .owner = THIS_MODULE, .read = wdm_read, .write = wdm_write, + .fsync = wdm_fsync, .open = wdm_open, .flush = wdm_flush, .release = wdm_release,
Greg Kroah-Hartman
maintainerPATCH 190/191 eeprom: at25: set minimum read/write access stride to 1
In reply to Greg Kroah-HartmanFrom: Christian Eggers <[email protected]>
commit 284f52ac1c6cfa1b2e5c11b84653dd90e4e91de7 upstream.
SPI eeproms are addressed by byte.
--- drivers/misc/eeprom/at25.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/misc/eeprom/at25.c +1 -1
@@ -355,7 +355,7 @@ static int at25_probe(struct spi_device at25->nvmem_config.reg_read = at25_ee_read; at25->nvmem_config.reg_write = at25_ee_write; at25->nvmem_config.priv = at25; - at25->nvmem_config.stride =4 ; + at25->nvmem_config.stride =1 ; at25->nvmem_config.word_size =1 ; at25->nvmem_config.size = chip.byte_len;
Greg Kroah-Hartman
maintainerPATCH 191/191 usb: gadget: f_ncm: allow using NCM in SuperSpeed Plus gadgets.
In reply to Greg Kroah-HartmanFrom: Lorenzo Colitti <[email protected]>
[ Upstream commit 7974ecd7d3c0f42a98566f281e44ea8573a2ad88 ]
Currently, enabling f_ncm at SuperSpeed Plus speeds results in an oops in config_ep_by_speed because ncm_set_alt passes in NULL ssp_descriptors. Fix this by re-using the SuperSpeed descriptors. This is safe because usb_assign_descriptors calls usb_copy_descriptors.
Tested: enabled f_ncm on a dwc3 gadget and 10Gbps link, ran iperf
--- drivers/usb/gadget/function/f_ncm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/function/f_ncm.c +1 -1
@@ -1541,7 +1541,7 @@ static int ncm_bind(struct usb_configuration *c, struct usb_function *f) fs_ncm_notify_desc.bEndpointAddress; status =usb_assign_descriptors (f, ncm_fs_function, ncm_hs_function, - ncm_ss_function,NULL ); + ncm_ss_function, ncm_ss_function);if (status) goto fail;
Signature · 1 line
2.25.1
A message that is not in the archive (sent off-list or not yet received): [email protected]
Retry.
Quoted 27 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 4.14.203 release. There are 191 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 29 Oct 2020 13:48:36 +0000. Anything received after that time might be too late.
Build results:
total: 168 pass: 166 fail: 2Failed builds:
powerpc:defconfig
powerpc:allmodconfig
Qemu test results:total: 404 pass: 385 fail: 19Failed tests:
<various powerpc64>Error log:
arch/powerpc/platforms/powernv/opal-dump.c: In function 'process_dump':arch/powerpc/platforms/powernv/opal-dump.c:409:7: error: void value not ignored as it ought to beTested-by Guenter Roeck <[email protected]>Guenter
Sasha Levin
maintainerQuoted 26 lines · Guenter Roeck
Retry.
Quoted 23 lines · Guenter Roeck
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 4.14.203 release. There are 191 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 29 Oct 2020 13:48:36 +0000. Anything received after that time might be too late.
Build results:
total: 168 pass: 166 fail: 2Failed builds:
powerpc:defconfig
powerpc:allmodconfig
Qemu test results:total: 404 pass: 385 fail: 19Failed tests:
<various powerpc64>Error log:
arch/powerpc/platforms/powernv/opal-dump.c: In function 'process_dump':arch/powerpc/platforms/powernv/opal-dump.c:409:7: error: void value not ignored as it ought to be
I'll grab b29336c0e178 ("powerpc/powernv/opal-dump : Use IRQ_HANDLED instead of numbers in interrupt handler") to fix that, thanks!
Signature · 2 lines
Thanks, Sasha
Quoted 10 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 4.14.203 release. There are 191 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 29 Oct 2020 13:48:36 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.203-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-4.14.y
and the diffstat can be found below.thanks,
greg k-h
Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.
Summary
------------------------------------------------------------------------
kernel: 4.14.203-rc1
git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
git branch: linux-4.14.y
git commit: a42f5f48c6ae133fc827daac1a64c70c300e32bb
git describe: v4.14.202-192-ga42f5f48c6ae
Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-4.14.y/build/v4.14.202-192-ga42f5f48c6ae
No regressions (compared to build v4.14.202)
No fixes (compared to build v4.14.202)
Ran 21826 total tests in the following environments and test suites.
Environments
--------------
- dragonboard-410c - arm64
- hi6220-hikey - arm64
- i386
- juno-r2 - arm64
- qemu_arm
- qemu_arm64
- qemu_i386
- qemu_x86_64
- x15 - arm
- x86_64
- x86-kasan
Test Suites
-----------
* build
* install-android-platform-tools-r2600
* linux-log-parser
* ltp-cap_bounds-tests
* ltp-commands-tests
* ltp-controllers-tests
* ltp-cpuhotplug-tests
* ltp-crypto-tests
* ltp-cve-tests
* ltp-fcntl-locktests-tests
* ltp-filecaps-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-math-tests
* ltp-mm-tests
* ltp-nptl-tests
* ltp-pty-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-tracing-tests
* perf
* ltp-dio-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-syscalls-tests
* network-basic-tests
* v4l2-compliance
* libhugetlbfs
* ltp-containers-tests
* ltp-fs-tests
Signature · 2 lines
Linaro LKFT https://lkft.linaro.org
Quoted 8 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 4.14.203 release. There are 191 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 29 Oct 2020 13:48:36 +0000. Anything received after that time might be too late.
Build reference: v4.14.202-192-ga42f5f48c6ae
gcc version: powerpc64-linux-gcc (GCC) 10.2.0
Building powerpc:defconfig ... failed
--------------
Error log:arch/powerpc/platforms/powernv/opal-dump.c: In function 'process_dump':arch/powerpc/platforms/powernv/opal-dump.c:409:7: error: void value not ignored as it ought to be
409 | dump = create_dump_obj(dump_id, dump_size, dump_type);
Full results will follow later.
Guenter
Quoted 9 lines · Greg Kroah-Hartman
This is the start of the stable review cycle for the 4.14.203 release. There are 191 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.
Responses should be made by Thu, 29 Oct 2020 13:48:36 +0000. Anything received after that time might be too late.
The whole patch series can be found in one patch at:
Quoted 8 lines
https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.203-rc1.gzor in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.gitlinux-4.14.y
and the diffstat can be found below.thanks,
greg k-h
All tests passing for Tegra ...
Test results for stable-v4.14:
8 builds: 8 pass, 0 fail
16 boots: 16 pass, 0 fail
30 tests: 30 pass, 0 failLinux version: 4.14.203-rc1-ga42f5f48c6ae
Boards tested: tegra124-jetson-tk1, tegra20-ventana,
tegra210-p2371-2180, tegra30-cardhu-a04
Jon