[PATCH v3] soc: qcom: pd-mapper: Fix element length in servreg_loc_pfr_req_ei

Mukesh Ojha posted 1 patch 1 month ago
drivers/soc/qcom/pdr_internal.h | 2 +-
drivers/soc/qcom/qcom_pdr_msg.c | 2 +-
include/linux/soc/qcom/pdr.h    | 1 +
3 files changed, 3 insertions(+), 2 deletions(-)
[PATCH v3] soc: qcom: pd-mapper: Fix element length in servreg_loc_pfr_req_ei
Posted by Mukesh Ojha 1 month ago
From: Gokul Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>

It looks element length declared in servreg_loc_pfr_req_ei for reason
not matching servreg_loc_pfr_req's reason field due which we could
observe decoding error on PD crash.

  qmi_decode_string_elem: String len 81 >= Max Len 65

Fix this by matching with servreg_loc_pfr_req's reason field.

Cc: stable@vger.kernel.org
Fixes: 1ebcde047c54 ("soc: qcom: add pd-mapper implementation")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Gokul Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
[mukesh: the element length change to the service field is not required.
 Fixed it by removing the change and rephrasing the commit text.]
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
Changes in v3: https://lore.kernel.org/lkml/20260202103641.3003867-1-mukesh.ojha@oss.qualcomm.com/
  - Remove debug patch as we have enough prints to make decode error
    code.
  - Added Gokul as the author of the patch and added the information on the 
    changes done by me on top.

Changes in v2: https://lore.kernel.org/lkml/20260129152320.3658053-1-mukesh.ojha@oss.qualcomm.com/ 
  - Given credit to my colleague Gokul.K who first faced this issue and given
    initial fix and that was later corrected by me.
  - Rebased it on next-20260130 and added stable mailing list, R-b tag.


 drivers/soc/qcom/pdr_internal.h | 2 +-
 drivers/soc/qcom/qcom_pdr_msg.c | 2 +-
 include/linux/soc/qcom/pdr.h    | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/pdr_internal.h b/drivers/soc/qcom/pdr_internal.h
index 039508c1bbf7..047c0160b617 100644
--- a/drivers/soc/qcom/pdr_internal.h
+++ b/drivers/soc/qcom/pdr_internal.h
@@ -84,7 +84,7 @@ struct servreg_set_ack_resp {
 
 struct servreg_loc_pfr_req {
 	char service[SERVREG_NAME_LENGTH + 1];
-	char reason[257];
+	char reason[SERVREG_PFR_LENGTH + 1];
 };
 
 struct servreg_loc_pfr_resp {
diff --git a/drivers/soc/qcom/qcom_pdr_msg.c b/drivers/soc/qcom/qcom_pdr_msg.c
index ca98932140d8..02022b11ecf0 100644
--- a/drivers/soc/qcom/qcom_pdr_msg.c
+++ b/drivers/soc/qcom/qcom_pdr_msg.c
@@ -325,7 +325,7 @@ const struct qmi_elem_info servreg_loc_pfr_req_ei[] = {
 	},
 	{
 		.data_type = QMI_STRING,
-		.elem_len = SERVREG_NAME_LENGTH + 1,
+		.elem_len = SERVREG_PFR_LENGTH + 1,
 		.elem_size = sizeof(char),
 		.array_type = VAR_LEN_ARRAY,
 		.tlv_type = 0x02,
diff --git a/include/linux/soc/qcom/pdr.h b/include/linux/soc/qcom/pdr.h
index 83a8ea612e69..2b7691e47c2a 100644
--- a/include/linux/soc/qcom/pdr.h
+++ b/include/linux/soc/qcom/pdr.h
@@ -5,6 +5,7 @@
 #include <linux/soc/qcom/qmi.h>
 
 #define SERVREG_NAME_LENGTH	64
+#define SERVREG_PFR_LENGTH	256
 
 struct pdr_service;
 struct pdr_handle;
-- 
2.50.1
Re: [PATCH v3] soc: qcom: pd-mapper: Fix element length in servreg_loc_pfr_req_ei
Posted by Nikita Travkin 1 month ago
Mukesh Ojha писал(а) 09.03.2026 14:01:
> From: Gokul Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
> 
> It looks element length declared in servreg_loc_pfr_req_ei for reason
> not matching servreg_loc_pfr_req's reason field due which we could
> observe decoding error on PD crash.
> 
>   qmi_decode_string_elem: String len 81 >= Max Len 65
> 
> Fix this by matching with servreg_loc_pfr_req's reason field.
> 

I've recently encountered same problem when causing a crash of slpi
on my Lenovo thinkpad x13s. This patch seems to fix it.

Before:
  [19146.069830] qmi_decode_string_elem: String len 87 >= Max Len 65
  [19146.069851] failed to decode incoming message
  [19146.085449] qcom_q6v5_pas 2400000.remoteproc: fatal error received: err_qdi.c:1122:EF:sensor_process:0x2:SNS_REG_TASK:0x62:sns_registry_sensor.c:154:sns_registry_sensor.c
  [19146.085462] remoteproc remoteproc0: crash detected in slpi: type fatal error

After:
  [   40.343732] PDM: service 'sensor_process' crash: 'EF:sensor_process:0x2:SNS_REG_TASK:0x62:sns_registry_s'
  [   40.358994] qcom_q6v5_pas 2400000.remoteproc: fatal error received: err_qdi.c:1122:EF:sensor_process:0x2:SNS_REG_TASK:0x62:sns_registry_sensor.c:154:sns_registry_sensor.c
  [   40.359012] remoteproc remoteproc0: crash detected in slpi: type fatal error

(There are two empty lines in dmesg now before the PDM: message, not
sure what writes them)

Tested-by: Nikita Travkin <nikita@trvn.ru>

Thanks!
Nikita

> Cc: stable@vger.kernel.org
> Fixes: 1ebcde047c54 ("soc: qcom: add pd-mapper implementation")
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Gokul Krishnakumar <gokul.krishnakumar@oss.qualcomm.com>
> [mukesh: the element length change to the service field is not required.
>  Fixed it by removing the change and rephrasing the commit text.]
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
> Changes in v3: https://lore.kernel.org/lkml/20260202103641.3003867-1-mukesh.ojha@oss.qualcomm.com/
>   - Remove debug patch as we have enough prints to make decode error
>     code.
>   - Added Gokul as the author of the patch and added the information on the 
>     changes done by me on top.
> 
> Changes in v2: https://lore.kernel.org/lkml/20260129152320.3658053-1-mukesh.ojha@oss.qualcomm.com/ 
>   - Given credit to my colleague Gokul.K who first faced this issue and given
>     initial fix and that was later corrected by me.
>   - Rebased it on next-20260130 and added stable mailing list, R-b tag.
> 
> 
>  drivers/soc/qcom/pdr_internal.h | 2 +-
>  drivers/soc/qcom/qcom_pdr_msg.c | 2 +-
>  include/linux/soc/qcom/pdr.h    | 1 +
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/qcom/pdr_internal.h b/drivers/soc/qcom/pdr_internal.h
> index 039508c1bbf7..047c0160b617 100644
> --- a/drivers/soc/qcom/pdr_internal.h
> +++ b/drivers/soc/qcom/pdr_internal.h
> @@ -84,7 +84,7 @@ struct servreg_set_ack_resp {
>  
>  struct servreg_loc_pfr_req {
>  	char service[SERVREG_NAME_LENGTH + 1];
> -	char reason[257];
> +	char reason[SERVREG_PFR_LENGTH + 1];
>  };
>  
>  struct servreg_loc_pfr_resp {
> diff --git a/drivers/soc/qcom/qcom_pdr_msg.c b/drivers/soc/qcom/qcom_pdr_msg.c
> index ca98932140d8..02022b11ecf0 100644
> --- a/drivers/soc/qcom/qcom_pdr_msg.c
> +++ b/drivers/soc/qcom/qcom_pdr_msg.c
> @@ -325,7 +325,7 @@ const struct qmi_elem_info servreg_loc_pfr_req_ei[] = {
>  	},
>  	{
>  		.data_type = QMI_STRING,
> -		.elem_len = SERVREG_NAME_LENGTH + 1,
> +		.elem_len = SERVREG_PFR_LENGTH + 1,
>  		.elem_size = sizeof(char),
>  		.array_type = VAR_LEN_ARRAY,
>  		.tlv_type = 0x02,
> diff --git a/include/linux/soc/qcom/pdr.h b/include/linux/soc/qcom/pdr.h
> index 83a8ea612e69..2b7691e47c2a 100644
> --- a/include/linux/soc/qcom/pdr.h
> +++ b/include/linux/soc/qcom/pdr.h
> @@ -5,6 +5,7 @@
>  #include <linux/soc/qcom/qmi.h>
>  
>  #define SERVREG_NAME_LENGTH	64
> +#define SERVREG_PFR_LENGTH	256
>  
>  struct pdr_service;
>  struct pdr_handle;