[PATCH] powerpc/spufs: Fix incorrect buffer offset in regs write

[PATCH] powerpc/spufs: Fix incorrect buffer offset in regs write

[Jeremy Kerr]

We need to offset by *pos bytes, not *pos words.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
 arch/powerpc/platforms/cell/spufs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index 83ef889..6b10877 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -578,7 +578,7 @@ spufs_regs_write(struct file *file, const char __user *buffer,
 	if (ret)
 		return ret;
 
-	ret = copy_from_user(lscsa->gprs + *pos - size,
+	ret = copy_from_user((char *)lscsa->gprs + *pos - size,
 			     buffer, size) ? -EFAULT : size;
 
 	spu_release_saved(ctx);

Re: [Cbe-oss-dev] [PATCH] powerpc/spufs: Fix incorrect buffer offset in regs write

[Geert Uytterhoeven]

On Wed, 4 Mar 2009, Jeremy Kerr wrote:
> We need to offset by *pos bytes, not *pos words.
> 
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> 
> ---
>  arch/powerpc/platforms/cell/spufs/file.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
> index 83ef889..6b10877 100644
> --- a/arch/powerpc/platforms/cell/spufs/file.c
> +++ b/arch/powerpc/platforms/cell/spufs/file.c
> @@ -578,7 +578,7 @@ spufs_regs_write(struct file *file, const char __user *buffer,
>  	if (ret)
>  		return ret;
>  
> -	ret = copy_from_user(lscsa->gprs + *pos - size,
> +	ret = copy_from_user((char *)lscsa->gprs + *pos - size,
>  			     buffer, size) ? -EFAULT : size;
>  
>  	spu_release_saved(ctx);

Could this be abused by an attacker to write registers or local store he's not
allowed to do?

Should it be backported to stable?

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: [Cbe-oss-dev] [PATCH] powerpc/spufs: Fix incorrect buffer offset in regs write

[Jeremy Kerr]

Hi Geert,

> Could this be abused by an attacker to write registers or local store
> he's not allowed to do?

It looks like the user can only overwrite fields that it already has 
access to. There's struct spu_lscsa:

struct spu_lscsa {
	struct spu_reg128 gprs[128];
	struct spu_reg128 fpcr;
	struct spu_reg128 decr;
	struct spu_reg128 decr_status;
	struct spu_reg128 ppu_mb;
	struct spu_reg128 ppuint_mb;
	struct spu_reg128 tag_mask;
	struct spu_reg128 event_mask;
	struct spu_reg128 srr0;
	struct spu_reg128 stopped_status;
	unsigned char ls[LS_SIZE] __attribute__((aligned(65536)));
};

where spu_reg128 is a u32[4].

The maximum 'allowed' write offset to the regs file is 2047. The 
(incorrect) maximum offset calculated by the old code would be 8188 
(2047 * 4) bytes into struct spu_lscsa.

So, 8188 bytes covers all of the registers, but ends somewhere before 
the start of the ls area (within the ls alignment padding). Let's look 
at the registers:

gprs:			user-writable
fpcr:			user-writable
decr:			user-writable
decr_status:	only affects user-settable SPE state
ppu_mb:		only affects user-settable SPE state
ppuint_mb:		only affects user-settable SPE state
tag_mask:		only affects user-settable SPE state
event_mask:	only affects user-settable SPE state
srr0:			only affects user-settable SPE state
stopped_status: only affects user-settable SPE state

So, I think we're fine. All a user can do with this bug is mess up their 
own SPE state.

> Should it be backported to stable?

Yes, I'll submit to the stable tree too.

Cheers,


Jeremy