arch: sparc: kernel: check the memory length before use strcpy().

PATCH Applied Chen Gang 2 participants 3 messages Jun 22, 2013 – Jul 10, 2013

Subsystems (2)

Generic include/asm header files SPARC + UltraSPARC (sparc/sparc64)

lore

Share & tools

lore

Chen Gang

author

2013-06-22 05:26 UTC #0 lore raw

PATCH 1/1 arch: sparc: kernel: check the memory length before use strcpy().

For the related next strcpy(), the destination length is less than 512, but the source maximize length may be 'OPROMMAXPARAM' (4096) which is more than 512.

One work flow may:
openprom_sunos_ioctl() -> if (cmd = OPROMSETOPT)

    getstrings() ->  will alloc buffer with size 'OPROMMAXPARAM'.
    opromsetopt() ->  devide the buffer into 'var' and 'value'
      of_set_property() -> pass
        prom_setprop() -> pass
          ldom_set_var()

And do not mind the additional 4 alignment buffer increasing, since 'sizeof(pkt) - sizeof(pkt.header)' is 4 alignment at least.

Signed-off-by Chen Gang <[email protected]>

---
 arch/sparc/kernel/ds.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/sparc/kernel/ds.c +10 -0

index 5ef48da..11d460f 100644
@@ -783,6 +783,16 @@ void ldom_set_var(const char *var, const char *value)
 		char  *base, *p;
 		int msg_len, loops;
 
+		if (strlen(var) + strlen(value) + 2 >
+		    sizeof(pkt) - sizeof(pkt.header)) {
+			printk(KERN_ERR PFX
+				"contents length: %zu, which more than max: %lu,"
+				"so could not set (%s) variable to (%s).\n",
+				strlen(var) + strlen(value) + 2,
+				sizeof(pkt) - sizeof(pkt.header), var, value);
+			return;
+		}
+
 		memset(&pkt, 0, sizeof(pkt));
 		pkt.header.data.tag.type = DS_DATA;
 		pkt.header.data.handle = cp->handle;

Signature · 1 line

1.7.11.7

David Miller

maintainer Applied

2013-07-10 20:42 UTC #1 lore raw

From: Chen Gang <[email protected]>
Date: Sat, 22 Jun 2013 13:26:09 +0800

Quoted 13 lines

For the related next strcpy(), the destination length is less than 512, but the source maximize length may be 'OPROMMAXPARAM' (4096) which is more than 512.
One work flow may:
openprom_sunos_ioctl() -> if (cmd == OPROMSETOPT)
    getstrings() ->  will alloc buffer with size 'OPROMMAXPARAM'.
    opromsetopt() ->  devide the buffer into 'var' and 'value'
      of_set_property() -> pass
        prom_setprop() -> pass
          ldom_set_var()

Quoted 5 lines

And do not mind the additional 4 alignment buffer increasing, since 'sizeof(pkt) - sizeof(pkt.header)' is 4 alignment at least.
Signed-off-by Chen Gang <[email protected]>

Applied.

Chen Gang

author

2013-07-10 23:41 UTC #2 lore raw

Quoted 16 lines · David Miller

From: Chen Gang <[email protected]>
Date: Sat, 22 Jun 2013 13:26:09 +0800
Quoted 13 lines
Quoted 13 lines
For the related next strcpy(), the destination length is less than 512, but the source maximize length may be 'OPROMMAXPARAM' (4096) which is more than 512.
One work flow may:
openprom_sunos_ioctl() -> if (cmd == OPROMSETOPT)
    getstrings() ->  will alloc buffer with size 'OPROMMAXPARAM'.
    opromsetopt() ->  devide the buffer into 'var' and 'value'
      of_set_property() -> pass
        prom_setprop() -> pass
          ldom_set_var()

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
And do not mind the additional 4 alignment buffer increasing, since 'sizeof(pkt) - sizeof(pkt.header)' is 4 alignment at least.
Signed-off-by Chen Gang <[email protected]>
Applied.

Thank you for your work, especially you are very busy.

Signature · 1 line

Chen Gang