Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] HID: Add quirk for HP X1200 PIXART OEM mouse
From: Jiri Kosina @ 2019-08-05 12:25 UTC (permalink / raw)
  To: Sebastian Parschauer; +Cc: Benjamin Tissoires, linux-input, stable
In-Reply-To: <20190724184003.12402-1-s.parschauer@gmx.de>

On Wed, 24 Jul 2019, Sebastian Parschauer wrote:

> The PixArt OEM mice are known for disconnecting every minute in
> runlevel 1 or 3 if they are not always polled. So add quirk
> ALWAYS_POLL for this one as well.
> 
> Jonathan Teh (@jonathan-teh) reported and tested the quirk.
> Reference: https://github.com/sriemer/fix-linux-mouse/issues/15
> 
> Signed-off-by: Sebastian Parschauer <s.parschauer@gmx.de>
> CC: stable@vger.kernel.org

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v2] HID: input: fix a4tech horizontal wheel custom usage
From: Jiri Kosina @ 2019-08-05 12:38 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: Benjamin Tissoires, dmitry.torokhov, wbauer, linux-input,
	linux-kernel
In-Reply-To: <cd69abeb3883ff7c7e2ff8dbe4db722f4e981875.camel@suse.de>

On Thu, 1 Aug 2019, Nicolas Saenz Julienne wrote:

> > Some a4tech mice use the 'GenericDesktop.00b8' usage to inform whether
> > the previous wheel report was horizontal or vertical. Before
> > c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key") this
> > usage was being mapped to 'Relative.Misc'. After the patch it's simply
> > ignored (usage->type == 0 & usage->code == 0). Which ultimately makes
> > hid-a4tech ignore the WHEEL/HWHEEL selection event, as it has no
> > usage->type.
> > 
> > We shouldn't rely on a mapping for that usage as it's nonstandard and
> > doesn't really map to an input event. So we bypass the mapping and make
> > sure the custom event handling properly handles both reports.
> > 
> > Fixes: c01908a14bf73 ("HID: input: add mapping for "Toggle Display" key")
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> 
> It would be nice for this patch not to get lost. It fixes issues both repoted
> on opensuse and fedora.

Sorry for the delay. I've now queued the patch. Thanks for fixing this,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* KASAN: slab-out-of-bounds Write in lg4ff_init
From: syzbot @ 2019-08-05 12:38 UTC (permalink / raw)
  To: andreyknvl, benjamin.tissoires, jikos, linux-input, linux-kernel,
	linux-usb, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=144c21dc600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
dashboard link: https://syzkaller.appspot.com/bug?extid=94e2b9e9c7d1dd332345
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=169e8542600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10ec8262600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+94e2b9e9c7d1dd332345@syzkaller.appspotmail.com

usb 1-1: config 0 interface 0 altsetting 0 has 1 endpoint descriptor,  
different from the interface descriptor's value: 9
usb 1-1: New USB device found, idVendor=046d, idProduct=c298, bcdDevice=  
0.00
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
logitech 0003:046D:C298.0001: unknown main item tag 0x0
logitech 0003:046D:C298.0001: unknown main item tag 0x0
logitech 0003:046D:C298.0001: hidraw0: USB HID v0.00 Device [HID 046d:c298]  
on usb-dummy_hcd.0-1/input0
==================================================================
BUG: KASAN: slab-out-of-bounds in set_bit  
include/asm-generic/bitops-instrumented.h:28 [inline]
BUG: KASAN: slab-out-of-bounds in lg4ff_init+0x89c/0x1800  
drivers/hid/hid-lg4ff.c:1331
Write of size 8 at addr ffff8881d81fe9c0 by task kworker/1:2/83

CPU: 1 PID: 83 Comm: kworker/1:2 Not tainted 5.3.0-rc2+ #25
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  print_address_description+0x6a/0x32c mm/kasan/report.c:351
  __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
  kasan_report+0xe/0x12 mm/kasan/common.c:612
  check_memory_region_inline mm/kasan/generic.c:185 [inline]
  check_memory_region+0x128/0x190 mm/kasan/generic.c:192
  set_bit include/asm-generic/bitops-instrumented.h:28 [inline]
  lg4ff_init+0x89c/0x1800 drivers/hid/hid-lg4ff.c:1331
  lg_probe+0x3b3/0x890 drivers/hid/hid-lg.c:850
  hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365
  usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  worker_thread+0x96/0xe20 kernel/workqueue.c:2415
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Allocated by task 83:
  save_stack+0x1b/0x80 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_kmalloc mm/kasan/common.c:487 [inline]
  __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
  kmalloc include/linux/slab.h:552 [inline]
  kzalloc include/linux/slab.h:748 [inline]
  hidraw_connect+0x4b/0x3e0 drivers/hid/hidraw.c:513
  hid_connect+0x5c7/0xbb0 drivers/hid/hid-core.c:1885
  hid_hw_start drivers/hid/hid-core.c:1981 [inline]
  hid_hw_start+0xa2/0x130 drivers/hid/hid-core.c:1972
  lg_probe+0x2a4/0x890 drivers/hid/hid-lg.c:806
  hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365
  usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  worker_thread+0x96/0xe20 kernel/workqueue.c:2415
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Freed by task 12:
  save_stack+0x1b/0x80 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
  slab_free_hook mm/slub.c:1423 [inline]
  slab_free_freelist_hook mm/slub.c:1470 [inline]
  slab_free mm/slub.c:3012 [inline]
  kfree+0xe4/0x2f0 mm/slub.c:3953
  blk_free_flush_queue+0x3f/0x4c block/blk-flush.c:500
  blk_mq_hw_sysfs_release+0x98/0x160 block/blk-mq-sysfs.c:43
  kobject_cleanup lib/kobject.c:693 [inline]
  kobject_release lib/kobject.c:722 [inline]
  kref_put include/linux/kref.h:65 [inline]
  kobject_put+0x171/0x280 lib/kobject.c:739
  blk_mq_release+0x258/0x3f0 block/blk-mq.c:2677
  __blk_release_queue+0x1ba/0x320 block/blk-sysfs.c:900
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  worker_thread+0x96/0xe20 kernel/workqueue.c:2415
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

The buggy address belongs to the object at ffff8881d81fe900
  which belongs to the cache kmalloc-192 of size 192
The buggy address is located 0 bytes to the right of
  192-byte region [ffff8881d81fe900, ffff8881d81fe9c0)
The buggy address belongs to the page:
page:ffffea0007607f80 refcount:1 mapcount:0 mapping:ffff8881da002a00  
index:0x0
flags: 0x200000000000200(slab)
raw: 0200000000000200 ffffea000760f000 0000000400000004 ffff8881da002a00
raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881d81fe880: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
  ffff8881d81fe900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8881d81fe980: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
                                            ^
  ffff8881d81fea00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8881d81fea80: 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH] Input: applespi - use struct_size() helper
From: Gustavo A. R. Silva @ 2019-08-06  0:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct touchpad_protocol {
	...
        struct tp_finger        fingers[0];
};

Make use of the struct_size() helper instead of an open-coded version
in order to avoid any potential type mistakes.

So, replace the following form:

sizeof(*tp) + tp->number_of_fingers * sizeof(tp->fingers[0]);

with:

struct_size(tp, fingers, tp->number_of_fingers)

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/input/keyboard/applespi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index acf34a5ff571..584289b67fb3 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1494,8 +1494,7 @@ static void applespi_got_data(struct applespi_data *applespi)
 		size_t tp_len;
 
 		tp = &message->touchpad;
-		tp_len = sizeof(*tp) +
-			 tp->number_of_fingers * sizeof(tp->fingers[0]);
+		tp_len = struct_size(tp, fingers, tp->number_of_fingers);
 
 		if (le16_to_cpu(message->length) + 2 != tp_len) {
 			dev_warn_ratelimited(&applespi->spi->dev,
-- 
2.22.0

^ permalink raw reply related

* Re: [PATCH 1/2] HID: hiddev: avoid opening a disconnected device
From: Jiri Kosina @ 2019-08-06  7:57 UTC (permalink / raw)
  To: Hillf Danton; +Cc: linux-input, linux-kernel, Andrey Konovalov
In-Reply-To: <20190806035820.10492-1-hdanton@sina.com>

On Tue, 6 Aug 2019, Hillf Danton wrote:

> In order to avoid opening a disconnected device, we need to check exist
> again after acquiring the existance lock, and bail out if necessary.
> 
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Hillf Danton <hdanton@sina.com>

Could you please add proper Reported-by: reference to syzbot? (in 2/2 as 
well).

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH resend 1/2] HID: hiddev: avoid opening a disconnected device
From: Jiri Kosina @ 2019-08-06 10:44 UTC (permalink / raw)
  To: Hillf Danton; +Cc: linux-input, linux-kernel, syzbot, Andrey Konovalov
In-Reply-To: <20190806083858.8032-1-hdanton@sina.com>

Both patches applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: sony: Fix race condition between rumble and device remove.
From: Jiri Kosina @ 2019-08-06 10:46 UTC (permalink / raw)
  To: Roderick Colenbrander
  Cc: linux-input, benjamin.tissoires, svv, pgriffais,
	Roderick Colenbrander, stable
In-Reply-To: <20190802225019.2418-1-roderick@gaikai.com>

On Fri, 2 Aug 2019, Roderick Colenbrander wrote:

> Valve reported a kernel crash on Ubuntu 18.04 when disconnecting a DS4
> gamepad while rumble is enabled. This issue is reproducible with a
> frequency of 1 in 3 times in the game Borderlands 2 when using an
> automatic weapon, which triggers many rumble operations.
> 
> We found the issue to be a race condition between sony_remove and the
> final device destruction by the HID / input system. The problem was
> that sony_remove didn't clean some of its work_item state in
> "struct sony_sc". After sony_remove work, the corresponding evdev
> node was around for sufficient time for applications to still queue
> rumble work after "sony_remove".
> 
> On pre-4.19 kernels the race condition caused a kernel crash due to a
> NULL-pointer dereference as "sc->output_report_dmabuf" got freed during
> sony_remove. On newer kernels this crash doesn't happen due the buffer
> now being allocated using devm_kzalloc. However we can still queue work,
> while the driver is an undefined state.
> 
> This patch fixes the described problem, by guarding the work_item
> "state_worker" with an initialized variable, which we are setting back
> to 0 on cleanup.

Applied to for-5.3/upstream-fixes. Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* KASAN: slab-out-of-bounds Read in usbhid_close
From: syzbot @ 2019-08-06 13:18 UTC (permalink / raw)
  To: andreyknvl, benjamin.tissoires, jikos, linux-input, linux-kernel,
	linux-usb, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=117a9f42600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
dashboard link: https://syzkaller.appspot.com/bug?extid=3268ee512f866a903602
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+3268ee512f866a903602@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: slab-out-of-bounds in __lock_acquire+0x302a/0x3b50  
kernel/locking/lockdep.c:3753
Read of size 8 at addr ffff8881ceab68a0 by task syz-executor.0/3352

CPU: 1 PID: 3352 Comm: syz-executor.0 Not tainted 5.3.0-rc2+ #25
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  print_address_description+0x6a/0x32c mm/kasan/report.c:351
  __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
  kasan_report+0xe/0x12 mm/kasan/common.c:612
  __lock_acquire+0x302a/0x3b50 kernel/locking/lockdep.c:3753
  lock_acquire+0x127/0x320 kernel/locking/lockdep.c:4412
  __raw_spin_lock_irq include/linux/spinlock_api_smp.h:128 [inline]
  _raw_spin_lock_irq+0x2d/0x40 kernel/locking/spinlock.c:167
  spin_lock_irq include/linux/spinlock.h:363 [inline]
  usbhid_close+0x51/0x210 drivers/hid/usbhid/hid-core.c:740
  hid_hw_close+0xa8/0xd0 drivers/hid/hid-core.c:2046
  drop_ref.part.0+0x32/0xe0 drivers/hid/hidraw.c:337
  drop_ref drivers/hid/hidraw.c:360 [inline]
  hidraw_release+0x34f/0x440 drivers/hid/hidraw.c:356
  __fput+0x2d7/0x840 fs/file_table.c:280
  task_work_run+0x13f/0x1c0 kernel/task_work.c:113
  exit_task_work include/linux/task_work.h:22 [inline]
  do_exit+0x8ef/0x2c50 kernel/exit.c:878
  do_group_exit+0x125/0x340 kernel/exit.c:982
  get_signal+0x466/0x23d0 kernel/signal.c:2728
  do_signal+0x88/0x14e0 arch/x86/kernel/signal.c:815
  exit_to_usermode_loop+0x1a2/0x200 arch/x86/entry/common.c:159
  prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
  syscall_return_slowpath arch/x86/entry/common.c:274 [inline]
  do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: Bad RIP value.
RSP: 002b:00007f123439dcf8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: fffffffffffffe00 RBX: 000000000075bf28 RCX: 0000000000459829
RDX: 0000000000000000 RSI: 0000000000000080 RDI: 000000000075bf28
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000075bf2c
R13: 00007ffe9281699f R14: 00007f123439e9c0 R15: 000000000075bf2c

Allocated by task 0:
(stack is not available)

Freed by task 0:
(stack is not available)

The buggy address belongs to the object at ffff8881ceab6880
  which belongs to the cache shmem_inode_cache of size 1168
The buggy address is located 32 bytes inside of
  1168-byte region [ffff8881ceab6880, ffff8881ceab6d10)
The buggy address belongs to the page:
page:ffffea00073aad00 refcount:1 mapcount:0 mapping:ffff8881da115180  
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000122 ffff8881da115180
raw: 0000000000000000 00000000800c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881ceab6780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8881ceab6800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881ceab6880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                                ^
  ffff8881ceab6900: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff8881ceab6980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

^ permalink raw reply

* KASAN: use-after-free Read in hiddev_ioctl
From: syzbot @ 2019-08-06 13:18 UTC (permalink / raw)
  To: andreyknvl, benjamin.tissoires, jikos, linux-input, linux-kernel,
	linux-usb, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=1732258a600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
dashboard link: https://syzkaller.appspot.com/bug?extid=5e9ed50a49eb77802d0e
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5e9ed50a49eb77802d0e@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: use-after-free in __mutex_lock_common  
kernel/locking/mutex.c:912 [inline]
BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360  
kernel/locking/mutex.c:1077
Read of size 8 at addr ffff8881cf955468 by task syz-executor.1/19529

CPU: 0 PID: 19529 Comm: syz-executor.1 Not tainted 5.3.0-rc2+ #25
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  print_address_description+0x6a/0x32c mm/kasan/report.c:351
  __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
  kasan_report+0xe/0x12 mm/kasan/common.c:612
  __mutex_lock_common kernel/locking/mutex.c:912 [inline]
  __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077
  hiddev_ioctl+0xea/0x1550 drivers/hid/usbhid/hiddev.c:607
  vfs_ioctl fs/ioctl.c:46 [inline]
  file_ioctl fs/ioctl.c:509 [inline]
  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
  __do_sys_ioctl fs/ioctl.c:720 [inline]
  __se_sys_ioctl fs/ioctl.c:718 [inline]
  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f0ec5dd7c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459829
RDX: 0000000020000380 RSI: 0000000081044804 RDI: 0000000000000005
RBP: 000000000075c070 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f0ec5dd86d4
R13: 00000000004c2249 R14: 00000000004d55f8 R15: 00000000ffffffff

Allocated by task 2777:
  save_stack+0x1b/0x80 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_kmalloc mm/kasan/common.c:487 [inline]
  __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
  kmalloc include/linux/slab.h:552 [inline]
  kzalloc include/linux/slab.h:748 [inline]
  hiddev_connect+0x242/0x5b0 drivers/hid/usbhid/hiddev.c:900
  hid_connect+0x239/0xbb0 drivers/hid/hid-core.c:1882
  hid_hw_start drivers/hid/hid-core.c:1981 [inline]
  hid_hw_start+0xa2/0x130 drivers/hid/hid-core.c:1972
  appleir_probe+0x13e/0x1a0 drivers/hid/hid-appleir.c:308
  hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365
  usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  process_scheduled_works kernel/workqueue.c:2331 [inline]
  worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Freed by task 2777:
  save_stack+0x1b/0x80 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
  slab_free_hook mm/slub.c:1423 [inline]
  slab_free_freelist_hook mm/slub.c:1470 [inline]
  slab_free mm/slub.c:3012 [inline]
  kfree+0xe4/0x2f0 mm/slub.c:3953
  hiddev_connect.cold+0x45/0x5c drivers/hid/usbhid/hiddev.c:914
  hid_connect+0x239/0xbb0 drivers/hid/hid-core.c:1882
  hid_hw_start drivers/hid/hid-core.c:1981 [inline]
  hid_hw_start+0xa2/0x130 drivers/hid/hid-core.c:1972
  appleir_probe+0x13e/0x1a0 drivers/hid/hid-appleir.c:308
  hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365
  usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  process_scheduled_works kernel/workqueue.c:2331 [inline]
  worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

The buggy address belongs to the object at ffff8881cf955400
  which belongs to the cache kmalloc-512 of size 512
The buggy address is located 104 bytes inside of
  512-byte region [ffff8881cf955400, ffff8881cf955600)
The buggy address belongs to the page:
page:ffffea00073e5500 refcount:1 mapcount:0 mapping:ffff8881da002500  
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 ffffea0007659680 0000000600000003 ffff8881da002500
raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881cf955300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881cf955380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881cf955400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                           ^
  ffff8881cf955480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881cf955500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

^ permalink raw reply

* Re: [PATCH] Input: applespi - use struct_size() helper
From: Dmitry Torokhov @ 2019-08-06 16:04 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: linux-input, linux-kernel
In-Reply-To: <20190806000638.GA4827@embeddedor>

On Mon, Aug 05, 2019 at 07:06:38PM -0500, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct touchpad_protocol {
> 	...
>         struct tp_finger        fingers[0];
> };
> 
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes.
> 
> So, replace the following form:
> 
> sizeof(*tp) + tp->number_of_fingers * sizeof(tp->fingers[0]);
> 
> with:
> 
> struct_size(tp, fingers, tp->number_of_fingers)
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Applied, thank you.

> ---
>  drivers/input/keyboard/applespi.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index acf34a5ff571..584289b67fb3 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
> @@ -1494,8 +1494,7 @@ static void applespi_got_data(struct applespi_data *applespi)
>  		size_t tp_len;
>  
>  		tp = &message->touchpad;
> -		tp_len = sizeof(*tp) +
> -			 tp->number_of_fingers * sizeof(tp->fingers[0]);
> +		tp_len = struct_size(tp, fingers, tp->number_of_fingers);
>  
>  		if (le16_to_cpu(message->length) + 2 != tp_len) {
>  			dev_warn_ratelimited(&applespi->spi->dev,
> -- 
> 2.22.0
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: docs: fix spelling mistake "potocol" -> "protocol"
From: Jonathan Corbet @ 2019-08-06 17:24 UTC (permalink / raw)
  To: Colin King
  Cc: Henrik Rydberg, Dmitry Torokhov, linux-input, linux-doc,
	kernel-janitors, linux-kernel
In-Reply-To: <20190805104951.26947-1-colin.king@canonical.com>

On Mon,  5 Aug 2019 11:49:51 +0100
Colin King <colin.king@canonical.com> wrote:

> There is a minor spelling mistake in the documentation, fix it.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  Documentation/input/multi-touch-protocol.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/input/multi-touch-protocol.rst b/Documentation/input/multi-touch-protocol.rst
> index 6be70342e709..307fe22d9668 100644
> --- a/Documentation/input/multi-touch-protocol.rst
> +++ b/Documentation/input/multi-touch-protocol.rst
> @@ -23,7 +23,7 @@ devices capable of tracking identifiable contacts (type B), the protocol
>  describes how to send updates for individual contacts via event slots.
>  
>  .. note::
> -   MT potocol type A is obsolete, all kernel drivers have been
> +   MT protocol type A is obsolete, all kernel drivers have been
>     converted to use type B.

Applied, thanks.

jon

^ permalink raw reply

* [PATCH] HID: wacom: Correct distance scale for 2nd-gen Intuos devices
From: Gerecke, Jason @ 2019-08-06 20:58 UTC (permalink / raw)
  To: linux-input, Jiri Kosina, Benjamin Tissoires
  Cc: Ping Cheng, Aaron Armstrong Skomra, Jason Gerecke, stable

From: Jason Gerecke <jason.gerecke@wacom.com>

Distance values reported by 2nd-gen Intuos tablets are on an inveted scale
(0 == far, 63 == near). We need to change them over to a normal scale before
reporting to userspace or else userspace drivers and applications can get
confused.

Ref: https://github.com/linuxwacom/input-wacom/issues/98
Fixes: eda01dab53 ("HID: wacom: Add four new Intuos devices")
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Cc: <stable@vger.kernel.org> # v4.4+
---
 drivers/hid/wacom_wac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 7a8ddc999a8e..879e41fbf604 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -846,6 +846,9 @@ static int wacom_intuos_general(struct wacom_wac *wacom)
 		y >>= 1;
 		distance >>= 1;
 	}
+	if (features->type == INTUOSHT2) {
+		distance = features->distance_max - distance;
+	}
 	input_report_abs(input, ABS_X, x);
 	input_report_abs(input, ABS_Y, y);
 	input_report_abs(input, ABS_DISTANCE, distance);
-- 
2.22.0

^ permalink raw reply related

* Re: [PATCH] HID: wacom: Correct distance scale for 2nd-gen Intuos devices
From: Greg KH @ 2019-08-07  5:18 UTC (permalink / raw)
  To: Gerecke, Jason
  Cc: linux-input, Jiri Kosina, Benjamin Tissoires, Ping Cheng,
	Aaron Armstrong Skomra, Jason Gerecke, stable
In-Reply-To: <20190806205805.21168-1-jason.gerecke@wacom.com>

On Tue, Aug 06, 2019 at 01:58:05PM -0700, Gerecke, Jason wrote:
> From: Jason Gerecke <jason.gerecke@wacom.com>
> 
> Distance values reported by 2nd-gen Intuos tablets are on an inveted scale
> (0 == far, 63 == near). We need to change them over to a normal scale before
> reporting to userspace or else userspace drivers and applications can get
> confused.
> 
> Ref: https://github.com/linuxwacom/input-wacom/issues/98
> Fixes: eda01dab53 ("HID: wacom: Add four new Intuos devices")
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Cc: <stable@vger.kernel.org> # v4.4+
> ---
>  drivers/hid/wacom_wac.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 7a8ddc999a8e..879e41fbf604 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -846,6 +846,9 @@ static int wacom_intuos_general(struct wacom_wac *wacom)
>  		y >>= 1;
>  		distance >>= 1;
>  	}
> +	if (features->type == INTUOSHT2) {
> +		distance = features->distance_max - distance;
> +	}

{ } not needed, always run checkpatch.pl before sending stuff out :(

^ permalink raw reply

* [PATCH v2] HID: wacom: Correct distance scale for 2nd-gen Intuos devices
From: Gerecke, Jason @ 2019-08-07 21:11 UTC (permalink / raw)
  To: linux-input, Jiri Kosina, Benjamin Tissoires
  Cc: Ping Cheng, Aaron Armstrong Skomra, Greg KH, Jason Gerecke,
	stable
In-Reply-To: <20190807051839.GA26833@kroah.com>

From: Jason Gerecke <jason.gerecke@wacom.com>

Distance values reported by 2nd-gen Intuos tablets are on an inverted
scale (0 == far, 63 == near). We need to change them over to a normal
scale before reporting to userspace or else userspace drivers and
applications can get confused.

Ref: https://github.com/linuxwacom/input-wacom/issues/98
Fixes: eda01dab53 ("HID: wacom: Add four new Intuos devices")
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Cc: <stable@vger.kernel.org> # v4.4+
---
Make checkpatch happy -- *doh!*

 drivers/hid/wacom_wac.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 7a8ddc999a8e..8e5063492242 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -846,6 +846,8 @@ static int wacom_intuos_general(struct wacom_wac *wacom)
 		y >>= 1;
 		distance >>= 1;
 	}
+	if (features->type == INTUOSHT2)
+		distance = features->distance_max - distance;
 	input_report_abs(input, ABS_X, x);
 	input_report_abs(input, ABS_Y, y);
 	input_report_abs(input, ABS_DISTANCE, distance);
-- 
2.22.0

^ permalink raw reply related

* [PATCH v2 0/3] HID: intel-ish-hid: support s2idle and s3 in ish_suspend()
From: Zhang Lixu @ 2019-08-08 10:21 UTC (permalink / raw)
  To: linux-input, jikos, benjamin.tissoires
  Cc: srinivas.pandruvada, linux-kernel, lixu.zhang

Currently, the NO_D3 flag is set in ish_probe(), the intel-ish-ipc driver
puts the ISH into D0i3 when system enter both suspend-to-idle(S0ix) and
suspend-to-mem(S3). These patches are to put the ISH into D3 when system
enter S3 and put the ISH into D0i3 when system enter S0ix.

I tested these patches on the following platforms:
ISH 5.2: ICL
ISH 5.0: CNL CFL WHL CML
ISH 4.0: GLK
ISH 3.0: KBL

Test steps:
* Run the IIO Sensor tool to read the accel_3d sensor data
* Run cmd "echo mem > /sys/power/state" to suspend-to-mem
* Press the keyboard to wake up OS.
* Check if the tool can get the sensor data after OS resume.
* Run cmd "echo freeze > /sys/power/state" to suspend-to-idle
* Press the keyboard to wake up OS.
* Check if the tool can get the sensor data after OS resume.

Test results:
The tool can get the accel_3d sensor data after resuming from both
suspend-to-mem and suspend-to-idle.

Changes from v1:
* Fix the indentation issue
* Elaborate the reason to remove the NO_D3 flag
* Split the PATCH v1 to three changes, and try to minimize the lines change

Zhang Lixu (3):
  HID: intel-ish-hid: ipc: set NO_D3 flag only when needed
  HID: intel-ish-hid: ipc: make ish suspend paths clear
  HID: intel-ish-hid: ipc: check the NO_D3 flag to distinguish resume
    paths

 drivers/hid/intel-ish-hid/ipc/hw-ish.h  |  1 +
 drivers/hid/intel-ish-hid/ipc/ipc.c     |  2 +-
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 95 +++++++++++++++----------
 3 files changed, 61 insertions(+), 37 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH v2 1/3] HID: intel-ish-hid: ipc: set NO_D3 flag only when needed
From: Zhang Lixu @ 2019-08-08 10:21 UTC (permalink / raw)
  To: linux-input, jikos, benjamin.tissoires
  Cc: srinivas.pandruvada, linux-kernel, lixu.zhang
In-Reply-To: <20190808102113.27802-1-lixu.zhang@intel.com>

Currently, the NO_D3 flag is set in ish_probe(), and cleared in
ish_remove(). So even if the system goes into S3, ISH is still
in D0i3 state. It makes more sense that put ISH into D3 as system
goes into S3 and put ISH into D0i3 as system goes into suspend-to-idle.
I remove the NO_D3 setting in ish_probe(), so that ISH can enter
D3 state when system enters S3. Only set N0_D3 flag when system
enters the suspend-to-idle or platform specified, and clear it
when system resume.

When the ISH enters D3, the FW will check the DMA bit status.
If the DMA bit is set, the FW will reset automatically. So the
DMA bit need be clear before putting ISH into D3 state.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
---
 drivers/hid/intel-ish-hid/ipc/hw-ish.h  |  1 +
 drivers/hid/intel-ish-hid/ipc/ipc.c     |  2 +-
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 21 +++++++++++++++++++--
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
index 1065692f90e2..ddd8e8e87cfa 100644
--- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
+++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
@@ -77,5 +77,6 @@ irqreturn_t ish_irq_handler(int irq, void *dev_id);
 struct ishtp_device *ish_dev_init(struct pci_dev *pdev);
 int ish_hw_start(struct ishtp_device *dev);
 void ish_device_disable(struct ishtp_device *dev);
+int ish_disable_dma(struct ishtp_device *dev);
 
 #endif /* _ISHTP_HW_ISH_H_ */
diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 18fe8af89aad..8f8dfdf64833 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -672,7 +672,7 @@ irqreturn_t ish_irq_handler(int irq, void *dev_id)
  *
  * Return: 0 for success else error code.
  */
-static int ish_disable_dma(struct ishtp_device *dev)
+int ish_disable_dma(struct ishtp_device *dev)
 {
 	unsigned int	dma_delay;
 
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index aa80b4d3b740..89b14d2edd0b 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -14,6 +14,7 @@
 #include <linux/types.h>
 #include <linux/pci.h>
 #include <linux/sched.h>
+#include <linux/suspend.h>
 #include <linux/interrupt.h>
 #include <linux/workqueue.h>
 #define CREATE_TRACE_POINTS
@@ -97,6 +98,11 @@ static const struct pci_device_id ish_invalid_pci_ids[] = {
 	{}
 };
 
+static inline bool ish_should_enter_d0i3(struct pci_dev *pdev)
+{
+	return !pm_suspend_via_firmware() || pdev->device == CHV_DEVICE_ID;
+}
+
 /**
  * ish_probe() - PCI driver probe callback
  * @pdev:	pci device
@@ -147,7 +153,6 @@ static int ish_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* mapping IO device memory */
 	hw->mem_addr = pcim_iomap_table(pdev)[0];
 	ishtp->pdev = pdev;
-	pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
 
 	/* request and enable interrupt */
 	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
@@ -184,7 +189,6 @@ static void ish_remove(struct pci_dev *pdev)
 	struct ishtp_device *ishtp_dev = pci_get_drvdata(pdev);
 
 	ishtp_bus_remove_all_clients(ishtp_dev, false);
-	pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
 	ish_device_disable(ishtp_dev);
 }
 
@@ -209,6 +213,8 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work)
 	uint32_t fwsts;
 	int ret;
 
+	pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
+
 	/* Get ISH FW status */
 	fwsts = IPC_GET_ISH_FWSTS(dev->ops->get_fw_status(dev));
 
@@ -267,6 +273,17 @@ static int __maybe_unused ish_suspend(struct device *device)
 						 !dev->suspend_flag,
 						  msecs_to_jiffies(25));
 
+	if (ish_should_enter_d0i3(pdev)) {
+		/* Set the NO_D3 flag, the ISH would enter D0i3 */
+		pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+	} else {
+		/*
+		 * Clear the DMA bit before putting ISH into D3,
+		 * or ISH FW would reset automatically.
+		 */
+		ish_disable_dma(dev);
+	}
+
 	return 0;
 }
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 2/3] HID: intel-ish-hid: ipc: make ish suspend paths clear
From: Zhang Lixu @ 2019-08-08 10:21 UTC (permalink / raw)
  To: linux-input, jikos, benjamin.tissoires
  Cc: srinivas.pandruvada, linux-kernel, lixu.zhang
In-Reply-To: <20190808102113.27802-1-lixu.zhang@intel.com>

For suspend-to-idle, send suspend message and set N0_D3 flag to put
the ISH into D0i3 state.
For suspend-to-mem, disable the DMA bit before ISH entering D3, and
NO_D3 flag is cleared by default, then the ISH would enter D3.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 52 +++++++++++++++----------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 89b14d2edd0b..35081f2cf781 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -223,6 +223,8 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work)
 	 * it means ISH isn't powered off, in this case, send a resume message.
 	 */
 	if (fwsts >= FWSTS_SENSOR_APP_LOADED) {
+		disable_irq_wake(pdev->irq);
+
 		ishtp_send_resume(dev);
 
 		/* Waiting to get resume response */
@@ -255,27 +257,36 @@ static int __maybe_unused ish_suspend(struct device *device)
 	struct pci_dev *pdev = to_pci_dev(device);
 	struct ishtp_device *dev = pci_get_drvdata(pdev);
 
-	enable_irq_wake(pdev->irq);
-	/*
-	 * If previous suspend hasn't been asnwered then ISH is likely dead,
-	 * don't attempt nested notification
-	 */
-	if (dev->suspend_flag)
-		return	0;
-
-	dev->resume_flag = 0;
-	dev->suspend_flag = 1;
-	ishtp_send_suspend(dev);
-
-	/* 25 ms should be enough for live ISH to flush all IPC buf */
-	if (dev->suspend_flag)
-		wait_event_interruptible_timeout(dev->suspend_wait,
-						 !dev->suspend_flag,
-						  msecs_to_jiffies(25));
-
 	if (ish_should_enter_d0i3(pdev)) {
-		/* Set the NO_D3 flag, the ISH would enter D0i3 */
-		pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+		/*
+		 * If previous suspend hasn't been asnwered then ISH is likely
+		 * dead, don't attempt nested notification
+		 */
+		if (dev->suspend_flag)
+			return	0;
+
+		dev->resume_flag = 0;
+		dev->suspend_flag = 1;
+		ishtp_send_suspend(dev);
+
+		/* 25 ms should be enough for live ISH to flush all IPC buf */
+		if (dev->suspend_flag)
+			wait_event_interruptible_timeout(dev->suspend_wait,
+					!dev->suspend_flag,
+					msecs_to_jiffies(25));
+
+		if (dev->suspend_flag) {
+			/*
+			 * It looks like FW halt, clear the DMA bit, and put
+			 * ISH into D3, and FW would reset on resume.
+			 */
+			ish_disable_dma(dev);
+		} else {
+			/* Set the NO_D3 flag, the ISH would enter D0i3 */
+			pdev->dev_flags |= PCI_DEV_FLAGS_NO_D3;
+
+			enable_irq_wake(pdev->irq);
+		}
 	} else {
 		/*
 		 * Clear the DMA bit before putting ISH into D3,
@@ -304,7 +315,6 @@ static int __maybe_unused ish_resume(struct device *device)
 	ish_resume_device = device;
 	dev->resume_flag = 1;
 
-	disable_irq_wake(pdev->irq);
 	schedule_work(&resume_work);
 
 	return 0;
-- 
2.17.1

^ permalink raw reply related

* [PATCH v2 3/3] HID: intel-ish-hid: ipc: check the NO_D3 flag to distinguish resume paths
From: Zhang Lixu @ 2019-08-08 10:21 UTC (permalink / raw)
  To: linux-input, jikos, benjamin.tissoires
  Cc: srinivas.pandruvada, linux-kernel, lixu.zhang
In-Reply-To: <20190808102113.27802-1-lixu.zhang@intel.com>

The NO_D3 flag would be set if the ISH enter D0i3 in ish_suspend(),
The resume paths can be distinguished by checking the NO_D3 flag.
It's more reasonable than checking the FW status.

Signed-off-by: Zhang Lixu <lixu.zhang@intel.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 34 +++++++++++--------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 35081f2cf781..f269852304e5 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -210,19 +210,11 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work)
 {
 	struct pci_dev *pdev = to_pci_dev(ish_resume_device);
 	struct ishtp_device *dev = pci_get_drvdata(pdev);
-	uint32_t fwsts;
 	int ret;
 
-	pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
-
-	/* Get ISH FW status */
-	fwsts = IPC_GET_ISH_FWSTS(dev->ops->get_fw_status(dev));
-
-	/*
-	 * If currently, in ISH FW, sensor app is loaded or beyond that,
-	 * it means ISH isn't powered off, in this case, send a resume message.
-	 */
-	if (fwsts >= FWSTS_SENSOR_APP_LOADED) {
+	/* Check the NO_D3 flag to distinguish the resume paths */
+	if (pdev->dev_flags & PCI_DEV_FLAGS_NO_D3) {
+		pdev->dev_flags &= ~PCI_DEV_FLAGS_NO_D3;
 		disable_irq_wake(pdev->irq);
 
 		ishtp_send_resume(dev);
@@ -232,16 +224,20 @@ static void __maybe_unused ish_resume_handler(struct work_struct *work)
 			ret = wait_event_interruptible_timeout(dev->resume_wait,
 				!dev->resume_flag,
 				msecs_to_jiffies(WAIT_FOR_RESUME_ACK_MS));
-	}
 
-	/*
-	 * If in ISH FW, sensor app isn't loaded yet, or no resume response.
-	 * That means this platform is not S0ix compatible, or something is
-	 * wrong with ISH FW. So on resume, full reboot of ISH processor will
-	 * happen, so need to go through init sequence again.
-	 */
-	if (dev->resume_flag)
+		/*
+		 * If the flag is not cleared, something is wrong with ISH FW.
+		 * So on resume, need to go through init sequence again.
+		 */
+		if (dev->resume_flag)
+			ish_init(dev);
+	} else {
+		/*
+		 * Resume from the D3, full reboot of ISH processor will happen,
+		 * so need to go through init sequence again.
+		 */
 		ish_init(dev);
+	}
 }
 
 /**
-- 
2.17.1

^ permalink raw reply related

* KASAN: use-after-free Read in usbhid_raw_request
From: syzbot @ 2019-08-08 12:38 UTC (permalink / raw)
  To: andreyknvl, benjamin.tissoires, jikos, linux-input, linux-kernel,
	linux-usb, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    e96407b4 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=16000516600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
dashboard link: https://syzkaller.appspot.com/bug?extid=75e6910bf03e266a277f
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+75e6910bf03e266a277f@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: use-after-free in usbhid_get_raw_report  
drivers/hid/usbhid/hid-core.c:865 [inline]
BUG: KASAN: use-after-free in usbhid_raw_request+0x5f2/0x640  
drivers/hid/usbhid/hid-core.c:1263
Read of size 8 at addr ffff8881c8702270 by task syz-executor.4/8993

CPU: 0 PID: 8993 Comm: syz-executor.4 Not tainted 5.3.0-rc2+ #25
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  print_address_description+0x6a/0x32c mm/kasan/report.c:351
  __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
  kasan_report+0xe/0x12 mm/kasan/common.c:612
  usbhid_get_raw_report drivers/hid/usbhid/hid-core.c:865 [inline]
  usbhid_raw_request+0x5f2/0x640 drivers/hid/usbhid/hid-core.c:1263
  hid_hw_raw_request include/linux/hid.h:1079 [inline]
  hidraw_get_report drivers/hid/hidraw.c:228 [inline]
  hidraw_ioctl+0x936/0xae0 drivers/hid/hidraw.c:426
  vfs_ioctl fs/ioctl.c:46 [inline]
  file_ioctl fs/ioctl.c:509 [inline]
  do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696
  ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
  __do_sys_ioctl fs/ioctl.c:720 [inline]
  __se_sys_ioctl fs/ioctl.c:718 [inline]
  __x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f7f49878c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459829
RDX: 0000000020000180 RSI: 00000000c0404807 RDI: 0000000000000004
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f7f498796d4
R13: 00000000004c2152 R14: 00000000004d54f8 R15: 00000000ffffffff

Allocated by task 83:
  save_stack+0x1b/0x80 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_kmalloc mm/kasan/common.c:487 [inline]
  __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
  kmalloc include/linux/slab.h:552 [inline]
  kzalloc include/linux/slab.h:748 [inline]
  usb_set_configuration+0x2c4/0x1670 drivers/usb/core/message.c:1846
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x650 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:882
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2114
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  worker_thread+0x96/0xe20 kernel/workqueue.c:2415
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Freed by task 83:
  save_stack+0x1b/0x80 mm/kasan/common.c:69
  set_track mm/kasan/common.c:77 [inline]
  __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
  slab_free_hook mm/slub.c:1423 [inline]
  slab_free_freelist_hook mm/slub.c:1470 [inline]
  slab_free mm/slub.c:3012 [inline]
  kfree+0xe4/0x2f0 mm/slub.c:3953
  device_release+0x71/0x200 drivers/base/core.c:1064
  kobject_cleanup lib/kobject.c:693 [inline]
  kobject_release lib/kobject.c:722 [inline]
  kref_put include/linux/kref.h:65 [inline]
  kobject_put+0x171/0x280 lib/kobject.c:739
  put_device+0x1b/0x30 drivers/base/core.c:2213
  usb_disable_device+0x2ce/0x690 drivers/usb/core/message.c:1244
  usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
  hub_port_connect drivers/usb/core/hub.c:4949 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  worker_thread+0x96/0xe20 kernel/workqueue.c:2415
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

The buggy address belongs to the object at ffff8881c8702200
  which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 112 bytes inside of
  2048-byte region [ffff8881c8702200, ffff8881c8702a00)
The buggy address belongs to the page:
page:ffffea000721c000 refcount:1 mapcount:0 mapping:ffff8881da00c000  
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000122 ffff8881da00c000
raw: 0000000000000000 00000000000f000f 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881c8702100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881c8702180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881c8702200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                              ^
  ffff8881c8702280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881c8702300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

^ permalink raw reply

* Re: [PATCH] HID: apple: Fix stuck function keys when using FN
From: João Moreno @ 2019-08-08 20:10 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <CAHxFc3QJ1Xkgckt1BPptXT5S1xkROVdJzHTYT=GAcHXgm5UGqg@mail.gmail.com>

Hi Benjamin,

On Mon, 8 Jul 2019 at 22:35, João Moreno <mail@joaomoreno.com> wrote:
>
> Hi Benjamin,
>
> No worries, also pretty busy over here. Didn't mean to press.
>
> On Mon, 1 Jul 2019 at 10:32, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > Hi João,
> >
> > On Sun, Jun 30, 2019 at 10:15 PM João Moreno <mail@joaomoreno.com> wrote:
> > >
> > > Hi Jiri & Benjamin,
> > >
> > > Let me know if you need something else to get this patch moving forward. This
> > > fixes an issue I hit daily, it would be great to get it fixed.
> >
> > Sorry for the delay, I am very busy with internal corporate stuff, and
> > I tried setting up a new CI system at home, and instead of spending a
> > couple of ours, I am down to 2 weeks of hard work, without possibility
> > to switch to the new right now :(
> > Anyway.
> >
> > >
> > > Thanks.
> > >
> > > On Mon, 10 Jun 2019 at 23:31, Joao Moreno <mail@joaomoreno.com> wrote:
> > > >
> > > > This fixes an issue in which key down events for function keys would be
> > > > repeatedly emitted even after the user has raised the physical key. For
> > > > example, the driver fails to emit the F5 key up event when going through
> > > > the following steps:
> > > > - fnmode=1: hold FN, hold F5, release FN, release F5
> > > > - fnmode=2: hold F5, hold FN, release F5, release FN
> >
> > Ouch :/
> >
>
> Right?!
>
> > > >
> > > > The repeated F5 key down events can be easily verified using xev.
> > > >
> > > > Signed-off-by: Joao Moreno <mail@joaomoreno.com>
> > > > ---
> > > >  drivers/hid/hid-apple.c | 21 +++++++++++----------
> > > >  1 file changed, 11 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > > > index 1cb41992aaa1..81867a6fa047 100644
> > > > --- a/drivers/hid/hid-apple.c
> > > > +++ b/drivers/hid/hid-apple.c
> > > > @@ -205,20 +205,21 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
> > > >                 trans = apple_find_translation (table, usage->code);
> > > >
> > > >                 if (trans) {
> > > > -                       if (test_bit(usage->code, asc->pressed_fn))
> > > > -                               do_translate = 1;
> > > > -                       else if (trans->flags & APPLE_FLAG_FKEY)
> > > > -                               do_translate = (fnmode == 2 && asc->fn_on) ||
> > > > -                                       (fnmode == 1 && !asc->fn_on);
> > > > +                       int fn_on = value ? asc->fn_on :
> > > > +                               test_bit(usage->code, asc->pressed_fn);
> > > > +
> > > > +                       if (!value)
> > > > +                               clear_bit(usage->code, asc->pressed_fn);
> > > > +                       else if (asc->fn_on)
> > > > +                               set_bit(usage->code, asc->pressed_fn);
> >
> > I have the feeling that this is not the correct fix here.
> >
> > I might be wrong, but the following sequence might also mess up the
> > driver state, depending on how the reports are emitted:
> > - hold FN, hold F4, hold F5, release F4, release FN, release F5
> >
>
> I believe this should be fine. Following the code:
>
> - hold FN, sets asc->fn_on to true
> - hold F4, in the trans block fn_on will be true and we'll set the F4
> bit in the bitmap
> - hold F5, in the trans block fn_on will be true and we'll set the F5 bit
> - release F4, in the trans block fn_on will be true (because of the bitmap) and
> we'll clear the F4 bit
> - release FN, asc->fn_on will be false, but it doesn't matter since...
> - release F5, in the trans block we'll look into the bitmap (instead
> of asc->fn_on),
> so fn_on will be true and we'll clear the F5 bit
>
> I tested it in practice using my changes:
>
> Interestingly the Apple keyboard doesn't seem to emit an even for F5 when F4 is
> pressed, seems like a hardware limitation. But F6 does work. So, when I execute
> these events in that order, everything works as it should: xev reports
> the following:
>
> KeyPress F4
> KeyPress F6
> KeyRelease F4
> KeyRelease F6
>
> > The reason is that the driver only considers you have one key pressed
> > with the modifier, and as the code changed its state based on the last
> > value.
> >
>
> I believe the bitmap takes care of storing the FN state per key press. The
> trick I did was to check on the global `asc->fn_on` state only when a key
> is pressed, but check on the bitmap instead when it's released.
>
> Let me know what you think. Am I missing something here?
>
> Cheers,
> João.
>
> > IMO a better fix would:
> >
> > - keep the existing `trans` mapping lookout
> > - whenever a `trans` mapping gets found:
> >   * get both translated and non-translated currently reported values
> > (`test_bit(keycode, input_dev->key)`)
> >   * if one of them is set to true, then consider the keycode to be the
> > one of the key (no matter fn_on)
> >     -> deal with `value` with the corrected keycode
> >   * if the key was not pressed:
> >     -> chose the keycode based on `fn_on` and `fnmode` states
> >     and report the key press event
> >
> > This should remove the nasty pressed_fn state which depends on the
> > other pressed keys.
> >
> > Cheers,
> > Benjamin
> >
> > > > +
> > > > +                       if (trans->flags & APPLE_FLAG_FKEY)
> > > > +                               do_translate = (fnmode == 2 && fn_on) ||
> > > > +                                       (fnmode == 1 && !fn_on);
> > > >                         else
> > > >                                 do_translate = asc->fn_on;
> > > >
> > > >                         if (do_translate) {
> > > > -                               if (value)
> > > > -                                       set_bit(usage->code, asc->pressed_fn);
> > > > -                               else
> > > > -                                       clear_bit(usage->code, asc->pressed_fn);
> > > > -
> > > >                                 input_event(input, usage->type, trans->to,
> > > >                                                 value);
> > > >
> > > > --
> > > > 2.19.1
> > > >

Gave this another look and I still haven't found any issues, let me
know if you still
think I'm missing something. Thanks!

Cheers,
João

^ permalink raw reply

* [PATCH 13/22] input: omap: void using mach/*.h headers
From: Arnd Bergmann @ 2019-08-08 21:22 UTC (permalink / raw)
  To: Tony Lindgren, Aaro Koskinen, Dmitry Torokhov
  Cc: linux-omap, linux-arm-kernel, Greg Kroah-Hartman, Linus Walleij,
	Bartlomiej Zolnierkiewicz, Tomi Valkeinen, Arnd Bergmann,
	linux-input, linux-kernel
In-Reply-To: <20190808212234.2213262-1-arnd@arndb.de>

By using the new linux/soc/ti/omap1-io.h header instead,
compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
conversion of omap1 may eventually be possible.

The warning in the header file gets removed in order to
allow CONFIG_COMPILE_TEST.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/input/keyboard/Kconfig            | 2 +-
 drivers/input/keyboard/omap-keypad.c      | 1 +
 include/linux/platform_data/keypad-omap.h | 5 -----
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 5f1a3b3ee0fb..b454d262906b 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -658,7 +658,7 @@ config KEYBOARD_IPAQ_MICRO
 
 config KEYBOARD_OMAP
 	tristate "TI OMAP keypad support"
-	depends on ARCH_OMAP1
+	depends on ARCH_OMAP1 || COMPILE_TEST
 	select INPUT_MATRIXKMAP
 	help
 	  Say Y here if you want to use the OMAP keypad.
diff --git a/drivers/input/keyboard/omap-keypad.c b/drivers/input/keyboard/omap-keypad.c
index 5fe7a5633e33..31da8e878535 100644
--- a/drivers/input/keyboard/omap-keypad.c
+++ b/drivers/input/keyboard/omap-keypad.c
@@ -24,6 +24,7 @@
 #include <linux/gpio.h>
 #include <linux/platform_data/gpio-omap.h>
 #include <linux/platform_data/keypad-omap.h>
+#include <linux/soc/ti/omap1-io.h>
 
 #undef NEW_BOARD_LEARNING_MODE
 
diff --git a/include/linux/platform_data/keypad-omap.h b/include/linux/platform_data/keypad-omap.h
index 3e7c64c854f4..6f058eb188c4 100644
--- a/include/linux/platform_data/keypad-omap.h
+++ b/include/linux/platform_data/keypad-omap.h
@@ -5,11 +5,6 @@
 #ifndef __KEYPAD_OMAP_H
 #define __KEYPAD_OMAP_H
 
-#ifndef CONFIG_ARCH_OMAP1
-#warning Please update the board to use matrix-keypad driver
-#define omap_readw(reg)		0
-#define omap_writew(val, reg)	do {} while (0)
-#endif
 #include <linux/input/matrix_keypad.h>
 
 struct omap_kp_platform_data {
-- 
2.20.0

^ permalink raw reply related

* Re: [PATCH 13/22] input: omap: void using mach/*.h headers
From: Dmitry Torokhov @ 2019-08-08 21:42 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Tony Lindgren, Aaro Koskinen, linux-omap, linux-arm-kernel,
	Greg Kroah-Hartman, Linus Walleij, Bartlomiej Zolnierkiewicz,
	Tomi Valkeinen, linux-input, linux-kernel
In-Reply-To: <20190808212234.2213262-14-arnd@arndb.de>

Hi Arnd,

On Thu, Aug 08, 2019 at 11:22:22PM +0200, Arnd Bergmann wrote:
> By using the new linux/soc/ti/omap1-io.h header instead,
> compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
> conversion of omap1 may eventually be possible.
> 
> The warning in the header file gets removed in order to
> allow CONFIG_COMPILE_TEST.

Given that we want to migrate people off this driver everywhere but
OMAP1 I wonder why we would want to improve compile coverage of it.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 13/22] input: omap: void using mach/*.h headers
From: Arnd Bergmann @ 2019-08-08 21:46 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Aaro Koskinen, Tony Lindgren, Greg Kroah-Hartman, Linus Walleij,
	Bartlomiej Zolnierkiewicz, Linux Kernel Mailing List,
	Tomi Valkeinen, open list:HID CORE LAYER, linux-omap, Linux ARM
In-Reply-To: <20190808214257.GF178933@dtor-ws>

On Thu, Aug 8, 2019 at 11:43 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Arnd,
>
> On Thu, Aug 08, 2019 at 11:22:22PM +0200, Arnd Bergmann wrote:
> > By using the new linux/soc/ti/omap1-io.h header instead,
> > compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
> > conversion of omap1 may eventually be possible.
> >
> > The warning in the header file gets removed in order to
> > allow CONFIG_COMPILE_TEST.
>
> Given that we want to migrate people off this driver everywhere but
> OMAP1 I wonder why we would want to improve compile coverage of it.

Mainly for consistency: I'm converting all omap1 drivers in this series to
not rely on mach/* headers and to let them be compiled standalone.
The other drivers don't have a replacement, so I could treat this different
from the rest and skip the Kconfig and platform_data changes if you
prefer.

        Arnd

^ permalink raw reply

* Re: [PATCH 13/22] input: omap: void using mach/*.h headers
From: Dmitry Torokhov @ 2019-08-08 22:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Tony Lindgren, Aaro Koskinen, linux-omap, Linux ARM,
	Greg Kroah-Hartman, Linus Walleij, Bartlomiej Zolnierkiewicz,
	Tomi Valkeinen, open list:HID CORE LAYER,
	Linux Kernel Mailing List
In-Reply-To: <CAK8P3a2TOcjxwCBGkZAhMAf9HuTL=FAB1e0=FAg+oHB0U1nJ0A@mail.gmail.com>

On Thu, Aug 08, 2019 at 11:46:45PM +0200, Arnd Bergmann wrote:
> On Thu, Aug 8, 2019 at 11:43 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Arnd,
> >
> > On Thu, Aug 08, 2019 at 11:22:22PM +0200, Arnd Bergmann wrote:
> > > By using the new linux/soc/ti/omap1-io.h header instead,
> > > compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
> > > conversion of omap1 may eventually be possible.
> > >
> > > The warning in the header file gets removed in order to
> > > allow CONFIG_COMPILE_TEST.
> >
> > Given that we want to migrate people off this driver everywhere but
> > OMAP1 I wonder why we would want to improve compile coverage of it.
> 
> Mainly for consistency: I'm converting all omap1 drivers in this series to
> not rely on mach/* headers and to let them be compiled standalone.
> The other drivers don't have a replacement, so I could treat this different
> from the rest and skip the Kconfig and platform_data changes if you
> prefer.

Yes, because at least with the version you posted we are losing the
#warning telling people to move to matrix_keypad. We could do:

#ifndef CONFIG_COMPILE_TEST
#warning ...
#endif

if you really want to allow compiling standalone for testing.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 13/22] input: omap: void using mach/*.h headers
From: Sebastian Reichel @ 2019-08-08 23:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Arnd Bergmann, Aaro Koskinen, Tony Lindgren, Greg Kroah-Hartman,
	Linus Walleij, Bartlomiej Zolnierkiewicz,
	Linux Kernel Mailing List, Tomi Valkeinen,
	open list:HID CORE LAYER, linux-omap, Linux ARM
In-Reply-To: <20190808221950.GG178933@dtor-ws>

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

Hi,

On Thu, Aug 08, 2019 at 03:19:50PM -0700, Dmitry Torokhov wrote:
> On Thu, Aug 08, 2019 at 11:46:45PM +0200, Arnd Bergmann wrote:
> > On Thu, Aug 8, 2019 at 11:43 PM Dmitry Torokhov wrote:
> > > On Thu, Aug 08, 2019 at 11:22:22PM +0200, Arnd Bergmann wrote:
> > > > By using the new linux/soc/ti/omap1-io.h header instead,
> > > > compile-testing can be enabled, and a CONFIG_ARCH_MULTIPLATFORM
> > > > conversion of omap1 may eventually be possible.
> > > >
> > > > The warning in the header file gets removed in order to
> > > > allow CONFIG_COMPILE_TEST.
> > >
> > > Given that we want to migrate people off this driver everywhere but
> > > OMAP1 I wonder why we would want to improve compile coverage of it.
> > 
> > Mainly for consistency: I'm converting all omap1 drivers in this series to
> > not rely on mach/* headers and to let them be compiled standalone.
> > The other drivers don't have a replacement, so I could treat this different
> > from the rest and skip the Kconfig and platform_data changes if you
> > prefer.
> 
> Yes, because at least with the version you posted we are losing the
> #warning telling people to move to matrix_keypad. We could do:
> 
> #ifndef CONFIG_COMPILE_TEST
> #warning ...
> #endif
> 
> if you really want to allow compiling standalone for testing.

FWIW the driver depends on ARCH_OMAP1 and the warning is
only printed for !ARCH_OMAP1. In other words: The warning
is never printed at the moment. All OMAP2+ boards moved to
matrix-keypad long time ago and the driver does not support
OMAP2+ anymore since f799a3d8fe170 from 2012.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox