On 20/03/2026 18:03, Casey Connolly wrote:
On 20/03/2026 17:54, David Heidelberg wrote:
On 20/03/2026 17:49, Casey Connolly wrote:
Hi David,
Nice timing with the series, I hit an OOB access (found it when I
enabled UBSAN) with this patch the other day.
The pdt_scan_state->pdts array should actually be of size
(RMI_PDT_MAX+1).
Additionally, I think rmi_pdt_entry_is_valid() is missing a bounds check.
Kind regards,
Thanks a lot for catching this and for the detailed notes — that’s very
helpful.
Since you’re the original author of the commit, I’m completely fine with
you taking over the b4 series if you’d prefer. Alternatively, if it’s
easier, feel free to just send me a fixed patch and I can incorporate it.
Uh sure, not sure this will apply cleanly I just edited inline it's a
3-line delta. Also figured we can drop pdt_count since it's unused.
The pdt_count is used in
Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
thus should be moved there I assume, but can be dropped here.
David
---
diff --git a/drivers/input/rmi4/rmi_driver.c
b/drivers/input/rmi4/rmi_driver.c
index ccd9338a44dbe..c7d2f68e65487 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -494,12 +494,39 @@ static void rmi_driver_copy_pdt_to_fd(const struct
pdt_entry *pdt,
fd->function_version = pdt->function_version;
}
+static bool rmi_pdt_entry_is_valid(struct rmi_device *rmi_dev,
+ struct pdt_scan_state *state, u8 fn)
+{
+ if (fn > RMI_PDT_MAX)
+ return false;
+
+ switch (fn) {
+ case 0x01:
+ case 0x03:
+ case 0x11:
+ case 0x12:
+ case 0x30:
+ case 0x34:
+ case 0x3a:
+ case 0x54:
+ case 0x55:
+ if (state->pdts[fn] == true)
+ return false;
+ break;
+ default:
+ rmi_dbg(RMI_DEBUG_CORE, &rmi_dev->dev,
+ "PDT has unknown function number %#02x\n", fn);
+ return false;
+ }
+
+ state->pdts[fn] = true;
+ return true;
+}
+
#define RMI_SCAN_CONTINUE 0
#define RMI_SCAN_DONE 1
static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
int page,
- int *empty_pages,
+ struct pdt_scan_state *state,
void *ctx,
int (*callback)(struct rmi_device *rmi_dev,
void *ctx,
@@ -522,6 +549,9 @@ static int rmi_scan_pdt_page(struct rmi_device *rmi_dev,
if (RMI4_END_OF_PDT(pdt_entry.function_number))
break;
+ if (!rmi_pdt_entry_is_valid(rmi_dev, state,
pdt_entry.function_number))
+ continue;
+
retval = callback(rmi_dev, ctx, &pdt_entry);
if (retval != RMI_SCAN_CONTINUE)
return retval;
@@ -532,11 +562,11 @@ static int rmi_scan_pdt_page(struct rmi_device
*rmi_dev,
* or more is found, stop scanning.
*/
if (addr == pdt_start)
- ++*empty_pages;
+ ++state->empty_pages;
else
- *empty_pages = 0;
+ state->empty_pages = 0;
- return (data->bootloader_mode || *empty_pages >= 2) ?
+ return (data->bootloader_mode || state->empty_pages >= 2) ?
RMI_SCAN_DONE : RMI_SCAN_CONTINUE;
}
@@ -545,11 +575,11 @@ int rmi_scan_pdt(struct rmi_device *rmi_dev, void
*ctx,
void *ctx, const struct pdt_entry *entry))
{
int page;
- int empty_pages = 0;
+ struct pdt_scan_state state = {0, {0}};
int retval = RMI_SCAN_DONE;
for (page = 0; page <= RMI4_MAX_PAGE; page++) {
- retval = rmi_scan_pdt_page(rmi_dev, page, &empty_pages,
+ retval = rmi_scan_pdt_page(rmi_dev, page, &state,
ctx, callback);
if (retval != RMI_SCAN_CONTINUE)
break;
diff --git a/drivers/input/rmi4/rmi_driver.h
b/drivers/input/rmi4/rmi_driver.h
index e84495caab151..a4ae2af93ce3a 100644
--- a/drivers/input/rmi4/rmi_driver.h
+++ b/drivers/input/rmi4/rmi_driver.h
@@ -46,6 +46,14 @@ struct pdt_entry {
u8 function_number;
};
+#define RMI_PDT_MAX 0x55
+
+struct pdt_scan_state {
+ u8 empty_pages;
+ bool pdts[RMI_PDT_MAX + 1];
+};
+
#define RMI_REG_DESC_PRESENSE_BITS (32 * BITS_PER_BYTE)
#define RMI_REG_DESC_SUBPACKET_BITS (37 * BITS_PER_BYTE)
--
David Heidelberg
