linux-stable.git/drivers/usb/gadget, branch linux-3.16.y

USB: gadget: fix illegal array access in binding with UDC

2020-06-11T18:05:53+00:00

commit 15753588bcd4bbffae1cca33c8ced5722477fe1f upstream.

FuzzUSB (a variant of syzkaller) found an illegal array access
using an incorrect index while binding a gadget with UDC.

Reference: https://www.spinics.net/lists/linux-usb/msg194331.html

This bug occurs when a size variable used for a buffer
is misused to access its strcpy-ed buffer.
Given a buffer along with its size variable (taken from user input),
from which, a new buffer is created using kstrdup().
Due to the original buffer containing 0 value in the middle,
the size of the kstrdup-ed buffer becomes smaller than that of the original.
So accessing the kstrdup-ed buffer with the same size variable
triggers memory access violation.

The fix makes sure no zero value in the buffer,
by comparing the strlen() of the orignal buffer with the size variable,
so that the access to the kstrdup-ed buffer is safe.

BUG: KASAN: slab-out-of-bounds in gadget_dev_desc_UDC_store+0x1ba/0x200
drivers/usb/gadget/configfs.c:266
Read of size 1 at addr ffff88806a55dd7e by task syz-executor.0/17208

CPU: 2 PID: 17208 Comm: syz-executor.0 Not tainted 5.6.8 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xce/0x128 lib/dump_stack.c:118
 print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374
 __kasan_report+0x131/0x1b0 mm/kasan/report.c:506
 kasan_report+0x12/0x20 mm/kasan/common.c:641
 __asan_report_load1_noabort+0x14/0x20 mm/kasan/generic_report.c:132
 gadget_dev_desc_UDC_store+0x1ba/0x200 drivers/usb/gadget/configfs.c:266
 flush_write_buffer fs/configfs/file.c:251 [inline]
 configfs_write_file+0x2f1/0x4c0 fs/configfs/file.c:283
 __vfs_write+0x85/0x110 fs/read_write.c:494
 vfs_write+0x1cd/0x510 fs/read_write.c:558
 ksys_write+0x18a/0x220 fs/read_write.c:611
 __do_sys_write fs/read_write.c:623 [inline]
 __se_sys_write fs/read_write.c:620 [inline]
 __x64_sys_write+0x73/0xb0 fs/read_write.c:620
 do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Signed-off-by: Kyungtae Kim 
Reported-and-tested-by: Kyungtae Kim 
Cc: Felipe Balbi 
Cc: stable 
Link: https://lore.kernel.org/r/20200510054326.GA19198@pizza01
Signed-off-by: Greg Kroah-Hartman 
Signed-off-by: Ben Hutchings

usb: gadget: f_ecm: Use atomic_t to track in-flight request

2020-05-22T20:19:27+00:00

commit d710562e01c48d59be3f60d58b7a85958b39aeda upstream.

Currently ecm->notify_req is used to flag when a request is in-flight.
ecm->notify_req is set to NULL and when a request completes it is
subsequently reset.

This is fundamentally buggy in that the unbind logic of the ECM driver will
unconditionally free ecm->notify_req leading to a NULL pointer dereference.

Fixes: da741b8c56d6 ("usb ethernet gadget: split CDC Ethernet function")
Signed-off-by: Bryan O'Donoghue 
Signed-off-by: Felipe Balbi 
Signed-off-by: Greg Kroah-Hartman 
[bwh: Backported to 3.16: adjust filename]
Signed-off-by: Ben Hutchings

usb: gadget: f_ncm: Use atomic_t to track in-flight request

2020-05-22T20:19:27+00:00

commit 5b24c28cfe136597dc3913e1c00b119307a20c7e upstream.

Currently ncm->notify_req is used to flag when a request is in-flight.
ncm->notify_req is set to NULL and when a request completes it is
subsequently reset.

This is fundamentally buggy in that the unbind logic of the NCM driver will
unconditionally free ncm->notify_req leading to a NULL pointer dereference.

Fixes: 40d133d7f542 ("usb: gadget: f_ncm: convert to new function interface with backward compatibility")
Signed-off-by: Bryan O'Donoghue 
Signed-off-by: Felipe Balbi 
Signed-off-by: Greg Kroah-Hartman 
[bwh: Backported to 3.16: adjust filename]
Signed-off-by: Ben Hutchings

usb: gadget: pch_udc: fix use after free

2020-02-11T20:03:32+00:00

commit 66d1b0c0580b7f1b1850ee4423f32ac42afa2e92 upstream.

Remove pointer dereference after free.

pci_pool_free doesn't care about contents of td.
It's just a void* for it

Addresses-Coverity-ID: 1091173 ("Use after free")
Acked-by: Michal Nazarewicz 
Signed-off-by: Gustavo A. R. Silva 
Link: https://lore.kernel.org/r/20191106202821.GA20347@embeddedor
Signed-off-by: Greg Kroah-Hartman 
[bwh: Backported to 3.16: adjust filename, context]
Signed-off-by: Ben Hutchings

usb: gadget: u_serial: add missing port entry locking

2020-02-11T20:03:19+00:00

commit daf82bd24e308c5a83758047aff1bd81edda4f11 upstream.

gserial_alloc_line() misses locking (for a release barrier) while
resetting port entry on TTY allocation failure. Fix this.

Signed-off-by: Michał Mirosław 
Reviewed-by: Greg Kroah-Hartman 
Tested-by: Ladislav Michl 
Signed-off-by: Felipe Balbi 
[bwh: Backported to 3.16: adjust filename]
Signed-off-by: Ben Hutchings

usb: gadget: rndis: free response queue during REMOTE_NDIS_RESET_MSG

2020-01-11T02:04:56+00:00

commit 207707d8fd48ebc977fb2b2794004a020e1ee08e upstream.

When rndis data transfer is in progress, some Windows7 Host PC is not
sending the GET_ENCAPSULATED_RESPONSE command for receiving the response
for the previous SEND_ENCAPSULATED_COMMAND processed.

The rndis function driver appends each response for the
SEND_ENCAPSULATED_COMMAND in a queue. As the above process got corrupted,
the Host sends a REMOTE_NDIS_RESET_MSG command to do a soft-reset.
As the rndis response queue is not freed, the previous response is sent
as a part of this REMOTE_NDIS_RESET_MSG's reset response and the Host
block any more Rndis transfers.

Hence free the rndis response queue as a part of this soft-reset so that
the correct response for REMOTE_NDIS_RESET_MSG is sent properly during the
response command.

Signed-off-by: Rajkumar Raghupathy 
Signed-off-by: Xerox Lin 
[AmitP: Cherry-picked this patch and folded other relevant
        fixes from Android common kernel android-4.4]
Signed-off-by: Amit Pundir 
Signed-off-by: Felipe Balbi 
Cc: Arnd Bergmann 
[bwh: Backported to 3.16:
 - Pass configNr instead of params as first argument to
   rndis_{get_next,free}_response()
 - Adjust filename, context]
Signed-off-by: Ben Hutchings

usb: gadget: configfs: add mutex lock before unregister gadget

2020-01-11T02:04:55+00:00

commit cee51c33f52ebf673a088a428ac0fecc33ab77fa upstream.

There may be a race condition if f_fs calls unregister_gadget_item in
ffs_closed() when unregister_gadget is called by UDC store at the same time.
this leads to a kernel NULL pointer dereference:

[  310.644928] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[  310.645053] init: Service 'adbd' is being killed...
[  310.658938] pgd = c9528000
[  310.662515] [00000004] *pgd=19451831, *pte=00000000, *ppte=00000000
[  310.669702] Internal error: Oops: 817 [#1] PREEMPT SMP ARM
[  310.675211] Modules linked in:
[  310.678294] CPU: 0 PID: 1537 Comm: ->transport Not tainted 4.1.15-03725-g793404c #2
[  310.685958] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[  310.692493] task: c8e24200 ti: c945e000 task.ti: c945e000
[  310.697911] PC is at usb_gadget_unregister_driver+0xb4/0xd0
[  310.703502] LR is at __mutex_lock_slowpath+0x10c/0x16c
[  310.708648] pc : []    lr : []    psr: 600f0113

[  311.565585] [] (usb_gadget_unregister_driver) from [] (unregister_gadget_item+0x1c/0x34)
[  311.575426] [] (unregister_gadget_item) from [] (ffs_closed+0x8c/0x9c)
[  311.583702] [] (ffs_closed) from [] (ffs_data_reset+0xc/0xa0)
[  311.591194] [] (ffs_data_reset) from [] (ffs_data_closed+0x90/0xd0)
[  311.599208] [] (ffs_data_closed) from [] (ffs_ep0_release+0xc/0x14)
[  311.607224] [] (ffs_ep0_release) from [] (__fput+0x80/0x1d0)
[  311.614635] [] (__fput) from [] (task_work_run+0xb0/0xe8)
[  311.621788] [] (task_work_run) from [] (do_work_pending+0x7c/0xa4)
[  311.629718] [] (do_work_pending) from [] (work_pending+0xc/0x20)

for functions using functionFS, i.e. android adbd will close /dev/usb-ffs/adb/ep0
when usb IO thread fails, but switch adb from on to off also triggers write
"none" > UDC. These 2 operations both call unregister_gadget, which will lead
to the panic above.

add a mutex before calling unregister_gadget for api used in f_fs.

Signed-off-by: Winter Wang 
Signed-off-by: Felipe Balbi 
Cc: Arnd Bergmann 
Signed-off-by: Ben Hutchings

usb: gadget: Add the gserial port checking in gs_start_tx()

2020-01-11T02:04:47+00:00

commit 511a36d2f357724312bb3776d2f6eed3890928b2 upstream.

When usb gadget is set gadget serial function, it will be crash in below
situation.

It will clean the 'port->port_usb' pointer in gserial_disconnect() function
when usb link is inactive, but it will release lock for disabling the endpoints
in this function. Druing the lock release period, it maybe complete one request
to issue gs_write_complete()--->gs_start_tx() function, but the 'port->port_usb'
pointer had been set NULL, thus it will be crash in gs_start_tx() function.

This patch adds the 'port->port_usb' pointer checking in gs_start_tx() function
to avoid this situation.

Signed-off-by: Baolin Wang 
Signed-off-by: Felipe Balbi 
Cc: Arnd Bergmann 
[bwh: Backported to 3.16: adjust filename]
Signed-off-by: Ben Hutchings

usb: gadget: serial: fix re-ordering of tx data

2020-01-11T02:04:46+00:00

commit 3e9d3d2efc677b501b12512cab5adb4f32a0673a upstream.

When a single thread is sending out data over the gadget serial port,
gs_start_tx() will be called both from the sender context and from the
write completion. Since the port lock is released before the packet is
queued, the order in which the URBs are submitted is not guaranteed.
E.g.

  sending thread                      completion (interrupt)

  gs_write()
    LOCK
                                      gs_write_complete()
                                        LOCK (wait)
    gs_start_tx()
      req1 = list_entry(pool->next)
      UNLOCK
                                        LOCK (acquired)
                                        gs_start_tx()
                                          req2 = list_entry(pool->next)
                                          UNLOCK
                                          usb_ep_queue(req2)
      usb_ep_queue(req1)

I.e., req2 is submitted before req1 but it contains the data that
comes after req1.

To reproduce, use SMP with sending thread and completion pinned to
different CPUs, or use PREEMPT_RT, and add the following delay just
before the call to usb_ep_queue():

		if (port->write_started > 0 && !list_empty(pool))
			udelay(1000);

To work around this problem, make sure that only one thread is running
through the gs_start_tx() loop with an extra flag write_busy. Since
gs_start_tx() is always called with the port lock held, no further
synchronisation is needed. The original caller will continue through
the loop when the request was successfully submitted.

Signed-off-by: Philip Oberstaller 
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) 
Signed-off-by: Felipe Balbi 
[bwh: Backported to 3.16: adjust filename]
Signed-off-by: Ben Hutchings

usb: gadget: composite: fix dereference after null check coverify warning

2020-01-11T02:04:46+00:00

commit c526c62d565ea5a5bba9433f28756079734f430d upstream.

cdev->config is checked for null pointer at above code, so cdev->config
might be null, fix it by adding null pointer check.

Signed-off-by: Peter Chen 
Signed-off-by: Felipe Balbi 
Cc: Arnd Bergmann 
Signed-off-by: Ben Hutchings