[PATCH v4 0/2] Input: synaptics-rmi4 - fix two device-controlled out-of-bounds writes

Hi Dmitry,

v4 is a tags-only respin: both patches carry the Assisted-by tags that
Documentation/process/coding-assistants.rst now asks for.  Neither diff
has changed since v3.  Sorry for the extra round trip.

The rest of this cover letter is unchanged from v3.

Two memory-safety problems in the shared RMI4 core, found while building
a reproducer for an out-of-bounds bug in hid-rmi [1].  Both are driven
entirely by data the *device* supplies -- its Page Description Table --
so they are reachable from a malicious USB HID device with no code
running on the victim, and equally from I2C and SMBus RMI4 devices.
Neither depends on the hid-rmi bug.  Both are present in mainline and in
every stable tree I looked at.

1/2 is an off-by-one: RMI_PDT_INT_SOURCE_COUNT_MASK is 0x07, so
interrupt_source_count can be 7, but struct rmi_function declares
int irq[RMI_FN_MAX_IRQS] with RMI_FN_MAX_IRQS == 6 and two loops walk it
up to fn->num_of_irqs.  irq[6] is the storage of the next member,
unsigned int irq_pos, so the function's position in the interrupt bitmap
is silently replaced with a Linux virq number.  UBSAN flags all five
stores plus the read on the unregister path.

2/2 is a time-of-check/time-of-use across two reads of the device: the
PDT is scanned three times and re-read from the device every time,
irq_mask[] is sized by the counting scan and filled by the creating scan,
and nothing verifies the two agree.  KASAN catches the resulting
set_bit() walking off the end of the flexible array at the tail of every
struct rmi_function.

2/2 also closes the device-driven route into an older problem in
rmi_create_function_irq(): irq_create_mapping() is called without
checking for failure, and it returns 0, not an error code.  A device that
declares one interrupt in the counting scan and four in the creating scan
makes the mapping fail, and irq_set_chip_data(0, fn) plus
irq_set_chip_and_handler(0, &rmi_irq_chip, handle_simple_irq) then
replace the chip and flow handler of IRQ 0 -- the timer, on x86:

  =========== /proc/interrupts, before ===========
    0:          9          0   IO-APIC   2-edge      timer
  =========== /proc/interrupts, after ============
    0:          9          0      rmi4   2  timer

  genirq: Flags mismatch irq 0. 00002000 (rmi4-00.fn01) vs. 00215a00 (timer)

With 2/2 applied the same device is rejected before any mapping is
attempted and IRQ 0 is untouched.  A standalone check of the
irq_create_mapping() return value still looks worthwhile, but it is a
separate change and I did not want to bury it in this series.

How this was verified:

Linux v6.12.69 (CONFIG_UBSAN_BOUNDS=y, booted slub_debug=FZPU) and
v6.12.105 (CONFIG_KASAN=y + CONFIG_KASAN_INLINE=y, CONFIG_UBSAN_BOUNDS=y,
booted kasan_multi_shot -- generic KASAN otherwise reports only the first
error per boot), x86_64.  An emulated Synaptics RMI4 device publishes a
Page Description Table crafted for each case, driven two ways with
identical results: a /dev/uhid program, and the same device over
dummy_hcd + raw-gadget with Facedancer so the reports really traverse
usbcore -> usbhid -> hid-rmi.  Each bug was exercised on its own cold
boot, because heap state left by a previous run changes what the
out-of-bounds read returns and UBSAN reports each call site only once
per boot.

With both patches applied 1/2 produces no UBSAN reports and a device
whose F01 declares the full 7 interrupt sources probes normally, and 2/2
fails the probe cleanly instead of corrupting the heap: three
consecutive runs of the growing-PDT device give three rejections
("F40: interrupt count changed between PDT scans (pos 1 + 6 > 1)",
"Function creation failed with code -22.") and zero KASAN reports,
against 14 from the unpatched core on the same boot, with no kobject or
refcount warnings; devices answering both scans consistently still probe
and report their real product id.

I am happy to post the reproducers, or to send them privately if you
would rather they did not go to a public list.

Changes in v4:

 - Assisted-by tags on both patches.  No code change.

Changes in v3:

 - 2/2: reject the function *before* allocating it, rather than
   disposing of it afterwards.  The v2 kfree() is correct on today's
   stable trees, but wrong on mainline since commit 58d42ec10b73
   ("Input: rmi4 - refactor function allocation and registration"):
   rmi_alloc_function() has since run device_initialize() and
   dev_set_name() on fn->dev, so kfree() there would leak the name
   string and skip the kobject cleanup.  put_device() is no alternative
   -- on the pre-refactor trees the stable backports target it warns and
   leaks exactly as v1 did.  Checking the counts before
   rmi_alloc_function() needs no cleanup on any tree; the error message
   and the -EINVAL are unchanged.
 - 1/2 unchanged.

Changes in v2:

 - 2/2: dispose of the rejected struct rmi_function with kfree() rather
   than put_device().  The rejection happens before
   rmi_register_function(), so device_initialize() has not run and
   fn->dev is still all zeroes; put_device() on it warned twice
   (kobject_put on an uninitialised kobject, then refcount underflow)
   and then leaked the function, because the saturated refcount stops
   the release from ever running.  ftrace over 405 rejections counted
   810 rmi_create_function against 405 rmi_release_function, matching a
   +407 growth in kmalloc-1k; with kfree() there are no warnings over
   206 rejections and the slab count is flat.  Thanks to the Sashiko
   automated review for prompting a closer look at that error path.
 - 1/2 unchanged.

The earlier postings are at
https://lore.kernel.org/linux-input/[email protected]/
and
https://lore.kernel.org/linux-input/[email protected]/
and
https://lore.kernel.org/linux-input/[email protected]/

[1] https://lore.kernel.org/linux-input/[email protected]/

Wei Jie Law (2):
  Input: synaptics-rmi4 - fix irq[] overrun with 7 interrupt sources
  Input: synaptics-rmi4 - reject a PDT that grows between scans

 drivers/input/rmi4/rmi_bus.h    |  9 ++++++---
 drivers/input/rmi4/rmi_driver.c | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

-- 
2.43.0