Re: [PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries

[David Heidelberg <david@ixit.cz>]

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