[PATCH] HID: rmi: fix OOB access with undersized RMI reports

[Wei Jie Law]

The hid-rmi driver sizes its writeReport/readReport buffer purely from
the report descriptor supplied by the device, with no minimum bound:

	data->input_report_size  = hid_report_len(input_report);
	data->output_report_size = hid_report_len(output_report);
	alloc_size = data->output_report_size + data->input_report_size;
	data->writeReport = devm_kzalloc(&hdev->dev, alloc_size, GFP_KERNEL);

but then writes fixed offsets into that buffer without validating the
sizes.  A device declaring a 1-byte output report (0x09) and a 1-byte
input report (0x0c) makes hid_report_len() return 2 for each, so
alloc_size is 4, and rmi_set_page() -- reached unconditionally at probe
time through rmi_input_configured() -- writes writeReport[4], one byte
past the allocation.  rmi_hid_read_block() writes writeReport[0..5] in
the same way, and rmi_hid_write_block() copies an unbounded len to
&writeReport[4].

The read path has the mirror image of the problem: the copy length
comes from readReport[1], which is filled in from the device's response
and can be up to 255, and the copy starts at &readReport[2] without any
regard for input_report_size, so it runs past the allocation and into
adjacent slab objects.  Those bytes become the register values the RMI
core acts on; they are printed to the kernel log as the product id by
rmi_f01_probe(), and they are sent back to the device as the interrupt
mask by rmi_driver_set_irq_bits(), so an undersized report descriptor
leaks heap contents both to userspace and to the device itself.

Reject reports that are too small at probe time, where the driver needs
6 output bytes for the write reports it builds and 3 input bytes for the
read handshake, and clamp the write and the read copy to the report
sizes the device declared.  A write longer than the output report was
already overrunning the buffer, so rejecting it cannot regress a device
that used to work.

Verified on v6.12.104, whose hid-rmi.c is identical to mainline here,
with a UHID reproducer and with an emulated USB device (raw-gadget): a
breakpoint on rmi_set_page() shows the store to writeReport[4] executing
with alloc_size == 4, and after the change probe stops with "rmi reports
too small (out=2 in=2)".  With a larger descriptor the read path returns
kernel heap bytes, observed both in the product id printed by
rmi_f01_probe() and in the interrupt mask written back to the emulated
device.

Fixes: 9fb6bf02e3ad ("HID: rmi: introduce RMI driver for Synaptics touchpads")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Wei Jie Law <98lawweijie@xxxxxxxxx>
---
The reproducer is available on request.

Compile-tested on bd5f485f3f02, x86_64 defconfig + CONFIG_HID_RMI=y.

 drivers/hid/hid-rmi.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index d4af17fdba46..a0780087d413 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -235,7 +235,8 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
 				break;
 			}
 
-			read_input_count = data->readReport[1];
+			read_input_count = min_t(int, data->readReport[1],
+						 data->input_report_size - 2);
 			memcpy(buf + bytes_read, &data->readReport[2],
 				min(read_input_count, bytes_needed));
 
@@ -271,6 +272,11 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
 			goto exit;
 	}
 
+	if (len > data->output_report_size - 4) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
 	data->writeReport[0] = RMI_WRITE_REPORT_ID;
 	data->writeReport[1] = len;
 	data->writeReport[2] = addr & 0xFF;
@@ -696,6 +702,17 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	data->output_report_size = hid_report_len(output_report);
 
+	/*
+	 * The write reports built by this driver occupy 6 bytes and the read
+	 * handshake looks at the first 3 bytes of an input report, so refuse
+	 * to drive a device whose reports cannot hold them.
+	 */
+	if (data->output_report_size < 6 || data->input_report_size < 3) {
+		hid_err(hdev, "rmi reports too small (out=%u in=%u)\n",
+			data->output_report_size, data->input_report_size);
+		goto start;
+	}
+
 	data->device_flags |= RMI_DEVICE;
 	alloc_size = data->output_report_size + data->input_report_size;
 

base-commit: bd5f485f3f026225b86573e559af0b7254ef4184
-- 
2.43.0