All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen@asianux.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	"paulus@samba.org" <paulus@samba.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: "sfr@canb.auug.org.au" <sfr@canb.auug.org.au>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [Suggestion] PowerPC: kernel: memory access violation when rtas_data_buf contents are more than 1026
Date: Thu, 18 Apr 2013 12:45:49 +0800	[thread overview]
Message-ID: <516F7A7D.60206@asianux.com> (raw)

Hello Maintainers:


in arch/powerpc/kernel/lparcfg.c, parse_system_parameter_string()

  need set '\0' for 'local_buffer'.

  the reason is:
    SPLPAR_MAXLENGTH is 1026, RTAS_DATA_BUF_SIZE is 4096
    the contents of rtas_data_buf may truncated in memcpy (line 301).

    if contents are truncated.
      the splpar_strlen is more than 1026 (line 321)
      the while loop checking will not find the end of buffer (line 326)
      it will cause memory access violation.


  I find it by reading code, so please help check.

  thanks.

gchen.

-------------------------related fix patch--------------------------------------

diff --git a/arch/powerpc/kernel/lparcfg.c b/arch/powerpc/kernel/lparcfg.c
index 801a757..d92f387 100644
--- a/arch/powerpc/kernel/lparcfg.c
+++ b/arch/powerpc/kernel/lparcfg.c
@@ -299,6 +299,7 @@ static void parse_system_parameter_string(struct seq_file *m)
 				__pa(rtas_data_buf),
 				RTAS_DATA_BUF_SIZE);
 	memcpy(local_buffer, rtas_data_buf, SPLPAR_MAXLENGTH);
+	local_buffer[SPLPAR_MAXLENGTH - 1] = '\0';
 	spin_unlock(&rtas_data_buf_lock);
 
 	if (call_status != 0) {



-------------------------related source code------------------------------------


283 static void parse_system_parameter_string(struct seq_file *m)
284 {
285         int call_status;
286 
287         unsigned char *local_buffer = kmalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
288         if (!local_buffer) {
289                 printk(KERN_ERR "%s %s kmalloc failure at line %d\n",
290                        __FILE__, __func__, __LINE__);
291                 return;
292         }
293 
294         spin_lock(&rtas_data_buf_lock);
295         memset(rtas_data_buf, 0, SPLPAR_MAXLENGTH);
296         call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
297                                 NULL,
298                                 SPLPAR_CHARACTERISTICS_TOKEN,
299                                 __pa(rtas_data_buf),
300                                 RTAS_DATA_BUF_SIZE);
301         memcpy(local_buffer, rtas_data_buf, SPLPAR_MAXLENGTH);
302         spin_unlock(&rtas_data_buf_lock);
303         
304         if (call_status != 0) {
305                 printk(KERN_INFO
306                        "%s %s Error calling get-system-parameter (0x%x)\n",
307                        __FILE__, __func__, call_status);
308         } else {       
309                 int splpar_strlen;
310                 int idx, w_idx;
311                 char *workbuffer = kzalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
312                 if (!workbuffer) { 
313                         printk(KERN_ERR "%s %s kmalloc failure at line %d\n",
314                                __FILE__, __func__, __LINE__);
315                         kfree(local_buffer);
316                         return;
317                 }       
318 #ifdef LPARCFG_DEBUG
319                 printk(KERN_INFO "success calling get-system-parameter\n");
320 #endif
321                 splpar_strlen = local_buffer[0] * 256 + local_buffer[1];
322                 local_buffer += 2;      /* step over strlen value */
323 
324                 w_idx = 0;
325                 idx = 0;
326                 while ((*local_buffer) && (idx < splpar_strlen)) {
327                         workbuffer[w_idx++] = local_buffer[idx++];
328                         if ((local_buffer[idx] == ',')
329                             || (local_buffer[idx] == '\0')) {
330                                 workbuffer[w_idx] = '\0';
331                                 if (w_idx) {
332                                         /* avoid the empty string */
333                                         seq_printf(m, "%s\n", workbuffer);
334                                 }
335                                 memset(workbuffer, 0, SPLPAR_MAXLENGTH);
336                                 idx++;  /* skip the comma */
337                                 w_idx = 0;
338                         } else if (local_buffer[idx] == '=') {
339                                 /* code here to replace workbuffer contents
340                                    with different keyword strings */
341                                 if (0 == strcmp(workbuffer, "MaxEntCap")) {
342                                         strcpy(workbuffer,
343                                                "partition_max_entitled_capacity");
344                                         w_idx = strlen(workbuffer);
345                                 }
346                                 if (0 == strcmp(workbuffer, "MaxPlatProcs")) {
347                                         strcpy(workbuffer,
348                                                "system_potential_processors");
349                                         w_idx = strlen(workbuffer);
350                                 }
351                         }
352                 }
353                 kfree(workbuffer);
354                 local_buffer -= 2;      /* back up over strlen value */
355         }
356         kfree(local_buffer);
357 }

WARNING: multiple messages have this Message-ID (diff)
From: Chen Gang <gang.chen@asianux.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	"paulus@samba.org" <paulus@samba.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michael Ellerman <michael@ellerman.id.au>,
	"sfr@canb.auug.org.au" <sfr@canb.auug.org.au>
Subject: [Suggestion] PowerPC: kernel: memory access violation when rtas_data_buf contents are more than 1026
Date: Thu, 18 Apr 2013 12:45:49 +0800	[thread overview]
Message-ID: <516F7A7D.60206@asianux.com> (raw)

Hello Maintainers:


in arch/powerpc/kernel/lparcfg.c, parse_system_parameter_string()

  need set '\0' for 'local_buffer'.

  the reason is:
    SPLPAR_MAXLENGTH is 1026, RTAS_DATA_BUF_SIZE is 4096
    the contents of rtas_data_buf may truncated in memcpy (line 301).

    if contents are truncated.
      the splpar_strlen is more than 1026 (line 321)
      the while loop checking will not find the end of buffer (line 326)
      it will cause memory access violation.


  I find it by reading code, so please help check.

  thanks.

gchen.

-------------------------related fix patch--------------------------------------

diff --git a/arch/powerpc/kernel/lparcfg.c b/arch/powerpc/kernel/lparcfg.c
index 801a757..d92f387 100644
--- a/arch/powerpc/kernel/lparcfg.c
+++ b/arch/powerpc/kernel/lparcfg.c
@@ -299,6 +299,7 @@ static void parse_system_parameter_string(struct seq_file *m)
 				__pa(rtas_data_buf),
 				RTAS_DATA_BUF_SIZE);
 	memcpy(local_buffer, rtas_data_buf, SPLPAR_MAXLENGTH);
+	local_buffer[SPLPAR_MAXLENGTH - 1] = '\0';
 	spin_unlock(&rtas_data_buf_lock);
 
 	if (call_status != 0) {



-------------------------related source code------------------------------------


283 static void parse_system_parameter_string(struct seq_file *m)
284 {
285         int call_status;
286 
287         unsigned char *local_buffer = kmalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
288         if (!local_buffer) {
289                 printk(KERN_ERR "%s %s kmalloc failure at line %d\n",
290                        __FILE__, __func__, __LINE__);
291                 return;
292         }
293 
294         spin_lock(&rtas_data_buf_lock);
295         memset(rtas_data_buf, 0, SPLPAR_MAXLENGTH);
296         call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
297                                 NULL,
298                                 SPLPAR_CHARACTERISTICS_TOKEN,
299                                 __pa(rtas_data_buf),
300                                 RTAS_DATA_BUF_SIZE);
301         memcpy(local_buffer, rtas_data_buf, SPLPAR_MAXLENGTH);
302         spin_unlock(&rtas_data_buf_lock);
303         
304         if (call_status != 0) {
305                 printk(KERN_INFO
306                        "%s %s Error calling get-system-parameter (0x%x)\n",
307                        __FILE__, __func__, call_status);
308         } else {       
309                 int splpar_strlen;
310                 int idx, w_idx;
311                 char *workbuffer = kzalloc(SPLPAR_MAXLENGTH, GFP_KERNEL);
312                 if (!workbuffer) { 
313                         printk(KERN_ERR "%s %s kmalloc failure at line %d\n",
314                                __FILE__, __func__, __LINE__);
315                         kfree(local_buffer);
316                         return;
317                 }       
318 #ifdef LPARCFG_DEBUG
319                 printk(KERN_INFO "success calling get-system-parameter\n");
320 #endif
321                 splpar_strlen = local_buffer[0] * 256 + local_buffer[1];
322                 local_buffer += 2;      /* step over strlen value */
323 
324                 w_idx = 0;
325                 idx = 0;
326                 while ((*local_buffer) && (idx < splpar_strlen)) {
327                         workbuffer[w_idx++] = local_buffer[idx++];
328                         if ((local_buffer[idx] == ',')
329                             || (local_buffer[idx] == '\0')) {
330                                 workbuffer[w_idx] = '\0';
331                                 if (w_idx) {
332                                         /* avoid the empty string */
333                                         seq_printf(m, "%s\n", workbuffer);
334                                 }
335                                 memset(workbuffer, 0, SPLPAR_MAXLENGTH);
336                                 idx++;  /* skip the comma */
337                                 w_idx = 0;
338                         } else if (local_buffer[idx] == '=') {
339                                 /* code here to replace workbuffer contents
340                                    with different keyword strings */
341                                 if (0 == strcmp(workbuffer, "MaxEntCap")) {
342                                         strcpy(workbuffer,
343                                                "partition_max_entitled_capacity");
344                                         w_idx = strlen(workbuffer);
345                                 }
346                                 if (0 == strcmp(workbuffer, "MaxPlatProcs")) {
347                                         strcpy(workbuffer,
348                                                "system_potential_processors");
349                                         w_idx = strlen(workbuffer);
350                                 }
351                         }
352                 }
353                 kfree(workbuffer);
354                 local_buffer -= 2;      /* back up over strlen value */
355         }
356         kfree(local_buffer);
357 }


             reply	other threads:[~2013-04-18  4:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-18  4:45 Chen Gang [this message]
2013-04-18  4:45 ` [Suggestion] PowerPC: kernel: memory access violation when rtas_data_buf contents are more than 1026 Chen Gang
2013-04-23  0:31 ` Benjamin Herrenschmidt
2013-04-23  0:31   ` Benjamin Herrenschmidt
2013-04-23  1:48   ` Chen Gang
2013-04-23  1:48     ` Chen Gang
2013-04-23  3:12     ` [PATCH] " Chen Gang
2013-04-23  3:12       ` Chen Gang
2013-04-24  6:28       ` Vasant Hegde
2013-04-24  6:28         ` Vasant Hegde
2013-04-24  7:03         ` Chen Gang
2013-04-24  7:03           ` Chen Gang
2013-04-24  7:23           ` Vasant Hegde
2013-04-24  7:23             ` Vasant Hegde
2013-04-24  7:40             ` Chen Gang
2013-04-24  7:40               ` Chen Gang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=516F7A7D.60206@asianux.com \
    --to=gang.chen@asianux.com \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    --cc=sfr@canb.auug.org.au \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.