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

[Casey Connolly]

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,

On 20/03/2026 17:44, David Heidelberg via B4 Relay wrote:
> From: Casey Connolly <casey.connolly@xxxxxxxxxx>
> 
> Some third party rmi4-compatible ICs don't expose their PDT entries
> very well. Add a few checks to skip duplicate entries as well as entries
> for unsupported functions.
> 
> This is required to support some phones with third party displays.
> 
> Validated on a stock OnePlus 6T (original parts):
> manufacturer: Synaptics, product: S3706B, fw id: 2852315
> 
> Co-developed-by: Kaustabh Chakraborty <kauschluss@xxxxxxxxxxx>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@xxxxxxxxxxx>
> Signed-off-by: Casey Connolly <casey.connolly@xxxxxxxxxx>
> Co-developed-by: David Heidelberg <david@xxxxxxx>
> Signed-off-by: David Heidelberg <david@xxxxxxx>
> ---
>  drivers/input/rmi4/rmi_driver.c | 42 +++++++++++++++++++++++++++++++++++------
>  drivers/input/rmi4/rmi_driver.h |  8 ++++++++
>  2 files changed, 44 insertions(+), 6 deletions(-)
> 
> 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)
> +{
> +	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;
> +	state->pdt_count++;
> +	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, {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;
> +	u8 pdt_count;
> +	bool pdts[RMI_PDT_MAX];
> +};
> +
>  #define RMI_REG_DESC_PRESENSE_BITS	(32 * BITS_PER_BYTE)
>  #define RMI_REG_DESC_SUBPACKET_BITS	(37 * BITS_PER_BYTE)
>  
> 

-- 
// Casey (she/her)