[PATCH] SN2: security hole in sn2_ptc_proc_write

[PATCH] SN2: security hole in sn2_ptc_proc_write

[Cliff Wickman]

From: Cliff Wickman <cpw@sgi.com>

Security hole in sn2_ptc_proc_write

It is possible to overrun a buffer with a write to this /proc file.

Diffed against 2.6.26-rc5

Signed-off-by: Cliff Wickman <cpw@sgi.com>
---
 arch/ia64/sn/kernel/sn2/sn2_smp.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6/arch/ia64/sn/kernel/sn2/sn2_smp.c
=================================--- linux-2.6.orig/arch/ia64/sn/kernel/sn2/sn2_smp.c
+++ linux-2.6/arch/ia64/sn/kernel/sn2/sn2_smp.c
@@ -512,6 +512,8 @@ static ssize_t sn2_ptc_proc_write(struct
 	int cpu;
 	char optstr[64];
 
+	if (count > 64)
+		return -EINVAL;
 	if (copy_from_user(optstr, user, count))
 		return -EFAULT;
 	optstr[count - 1] = '\0';

Re: [PATCH] SN2: security hole in sn2_ptc_proc_write

[Andi Kleen]

Cliff Wickman <cpw@sgi.com> writes:
>
> Signed-off-by: Cliff Wickman <cpw@sgi.com>
> ---
>  arch/ia64/sn/kernel/sn2/sn2_smp.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> Index: linux-2.6/arch/ia64/sn/kernel/sn2/sn2_smp.c
> =================================> --- linux-2.6.orig/arch/ia64/sn/kernel/sn2/sn2_smp.c
> +++ linux-2.6/arch/ia64/sn/kernel/sn2/sn2_smp.c
> @@ -512,6 +512,8 @@ static ssize_t sn2_ptc_proc_write(struct
>  	int cpu;
>  	char optstr[64];
>  
> +	if (count > 64)
> +		return -EINVAL;
>  	if (copy_from_user(optstr, user, count))
>  		return -EFAULT;
>  	optstr[count - 1] = '\0';


When someone passes 0 you get an buffer underflow? 

You need if (count < 0 || count > 64) me thinks.

-Andi

Re: [PATCH] SN2: security hole in sn2_ptc_proc_write

[Bernhard Walle]

* Andi Kleen <andi@firstfloor.org> [2008-06-22 19:41]:
>
> You need if (count < 0 || count > 64) me thinks.

size_t is unsigned.


Bernhard
-- 
Bernhard Walle, SUSE LINUX Products GmbH, Architecture Development

Re: [PATCH] SN2: security hole in sn2_ptc_proc_write

[Petr Tesarik]

On Sun, 2008-06-22 at 20:58 +0200, Bernhard Walle wrote:
> * Andi Kleen <andi@firstfloor.org> [2008-06-22 19:41]:
> >
> > You need if (count < 0 || count > 64) me thinks.
> 
> size_t is unsigned.

Correct, that's why you need:

  if (count <= 0 || count > 64)

Cheers,
Petr Tesarik

Re: [PATCH] SN2: security hole in sn2_ptc_proc_write

[Andi Kleen]

Bernhard Walle wrote:
> * Andi Kleen <andi@firstfloor.org> [2008-06-22 19:41]:
>> You need if (count < 0 || count > 64) me thinks.
> 
> size_t is unsigned.

Yes = or <= sorry. 

-Andi