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

David Heidelberg via B4 Relay posted 7 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
Posted by David Heidelberg via B4 Relay 2 weeks, 2 days ago
From: Casey Connolly <casey.connolly@linaro.org>

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@disroot.org>
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
Co-developed-by: David Heidelberg <david@ixit.cz>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
 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)
 

-- 
2.53.0
Re: [PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
Posted by Casey Connolly 2 weeks, 2 days ago
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@linaro.org>
> 
> 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@disroot.org>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> Co-developed-by: David Heidelberg <david@ixit.cz>
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  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)
Re: [PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
Posted by David Heidelberg 2 weeks, 2 days ago
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.

Whichever works best for you.

David>
> On 20/03/2026 17:44, David Heidelberg via B4 Relay wrote:
>> From: Casey Connolly <casey.connolly@linaro.org>
>>
>> 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@disroot.org>
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>> Co-developed-by: David Heidelberg <david@ixit.cz>
>> Signed-off-by: David Heidelberg <david@ixit.cz>

[...]
Re: [PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
Posted by Casey Connolly 2 weeks, 2 days ago

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.

---

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)


-- 
2.53.0




-- 
// Casey (she/her)

Re: [PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
Posted by David Heidelberg 2 weeks, 2 days ago
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

Re: [PATCH v7 2/7] Input: synaptics-rmi4 - handle duplicate/unknown PDT entries
Posted by David Heidelberg 2 weeks, 2 days ago
Pushed changes to

https://codeberg.org/sdm845/linux/commits/branch/b4/synaptics-rmi4

On 20/03/2026 18:12, David Heidelberg wrote:
> 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