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

[David Heidelberg <david@ixit.cz>]

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