[PATCH] powerpc/pseries/lparcfg: size the scratch buffer to the system parameter payload

[PATCH] powerpc/pseries/lparcfg: size the scratch buffer to the system parameter payload

[Pengpeng Hou]

parse_system_parameter_string() reads the shared processor LPAR
attributes into a firmware buffer that can hold up to 4000 bytes, but it
still tokenizes that payload through a fixed 1026-byte scratch buffer. A
single long key-value fragment can therefore overrun the local parser
buffer before the next comma delimiter is seen.

Allocate the scratch buffer to the current payload size so tokenization
stays within bounds.

Fixes: fff9846be00c ("powerpc/pseries/lparcfg: convert to papr_sysparm API")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 arch/powerpc/platforms/pseries/lparcfg.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index 8821c378bfff..c09f474c241e 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -385,8 +385,6 @@ static void read_lpar_name(struct seq_file *m)
 		read_dt_lpar_name(m);
 }
 
-#define SPLPAR_MAXLENGTH 1026*(sizeof(char))
-
 /*
  * parse_system_parameter_string()
  * Retrieve the potential_processors, max_entitled_capacity and friends
@@ -407,27 +405,32 @@ static void parse_system_parameter_string(struct seq_file *m)
 		const char *local_buffer;
 		int splpar_strlen;
 		int idx, w_idx;
-		char *workbuffer = kzalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
-
-		if (!workbuffer)
-			goto out_free;
+		size_t workbuf_size;
+		char *workbuffer;
 
 		splpar_strlen = be16_to_cpu(buf->len);
 		local_buffer = buf->val;
+		workbuf_size = splpar_strlen + 1;
+
+		workbuffer = kzalloc(workbuf_size, GFP_KERNEL);
+		if (!workbuffer)
+			goto out_free;
 
 		w_idx = 0;
 		idx = 0;
-		while ((*local_buffer) && (idx < splpar_strlen)) {
+		while ((idx < splpar_strlen) && local_buffer[idx]) {
 			workbuffer[w_idx++] = local_buffer[idx++];
-			if ((local_buffer[idx] == ',')
+			if (idx >= splpar_strlen ||
+			    (local_buffer[idx] == ',')
 			    || (local_buffer[idx] == '\0')) {
 				workbuffer[w_idx] = '\0';
 				if (w_idx) {
 					/* avoid the empty string */
 					seq_printf(m, "%s\n", workbuffer);
 				}
-				memset(workbuffer, 0, SPLPAR_MAXLENGTH);
-				idx++;	/* skip the comma */
+				memset(workbuffer, 0, workbuf_size);
+				if (idx < splpar_strlen)
+					idx++;	/* skip the comma */
 				w_idx = 0;
 			} else if (local_buffer[idx] == '=') {
 				/* code here to replace workbuffer contents
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] powerpc/pseries/lparcfg: size the scratch buffer to the system parameter payload

[R Nageswara Sastry]

On 01.04.2026 9:33 PM, Pengpeng Hou wrote:
> parse_system_parameter_string() reads the shared processor LPAR
> attributes into a firmware buffer that can hold up to 4000 bytes, but it
> still tokenizes that payload through a fixed 1026-byte scratch buffer. A
> single long key-value fragment can therefore overrun the local parser
> buffer before the next comma delimiter is seen.
>
> Allocate the scratch buffer to the current payload size so tokenization
> stays within bounds.
>
> Fixes: fff9846be00c ("powerpc/pseries/lparcfg: convert to papr_sysparm API")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---

Tested-by: R Nageswara Sastry <rnsastry@linux.ibm.com>

Tested with different sizes of the buffer namely 1000, 1026, 1500, 2000, 
3900, 4000, 1027, 3500 with a sample test kernel module. Using the same 
module injected the text with the above sizes in to lparcfg
Example:

system_potential_processors=8HIJKLMN...GHIJK

>   arch/powerpc/platforms/pseries/lparcfg.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
> index 8821c378bfff..c09f474c241e 100644
> --- a/arch/powerpc/platforms/pseries/lparcfg.c
> +++ b/arch/powerpc/platforms/pseries/lparcfg.c
> @@ -385,8 +385,6 @@ static void read_lpar_name(struct seq_file *m)
>   		read_dt_lpar_name(m);
>   }
>   
> -#define SPLPAR_MAXLENGTH 1026*(sizeof(char))
> -
>   /*
>    * parse_system_parameter_string()
>    * Retrieve the potential_processors, max_entitled_capacity and friends
> @@ -407,27 +405,32 @@ static void parse_system_parameter_string(struct seq_file *m)
>   		const char *local_buffer;
>   		int splpar_strlen;
>   		int idx, w_idx;
> -		char *workbuffer = kzalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
> -
> -		if (!workbuffer)
> -			goto out_free;
> +		size_t workbuf_size;
> +		char *workbuffer;
>   
>   		splpar_strlen = be16_to_cpu(buf->len);
>   		local_buffer = buf->val;
> +		workbuf_size = splpar_strlen + 1;
> +
> +		workbuffer = kzalloc(workbuf_size, GFP_KERNEL);
> +		if (!workbuffer)
> +			goto out_free;
>   
>   		w_idx = 0;
>   		idx = 0;
> -		while ((*local_buffer) && (idx < splpar_strlen)) {
> +		while ((idx < splpar_strlen) && local_buffer[idx]) {
>   			workbuffer[w_idx++] = local_buffer[idx++];
> -			if ((local_buffer[idx] == ',')
> +			if (idx >= splpar_strlen ||
> +			    (local_buffer[idx] == ',')
>   			    || (local_buffer[idx] == '\0')) {
>   				workbuffer[w_idx] = '\0';
>   				if (w_idx) {
>   					/* avoid the empty string */
>   					seq_printf(m, "%s\n", workbuffer);
>   				}
> -				memset(workbuffer, 0, SPLPAR_MAXLENGTH);
> -				idx++;	/* skip the comma */
> +				memset(workbuffer, 0, workbuf_size);
> +				if (idx < splpar_strlen)
> +					idx++;	/* skip the comma */
>   				w_idx = 0;
>   			} else if (local_buffer[idx] == '=') {
>   				/* code here to replace workbuffer contents

-- 
Thanks and Regards
R.Nageswara Sastry