64bit kernel/N32 userspace - shmctl corrupts userspace memory

64bit kernel/N32 userspace - shmctl corrupts userspace memory

[Chad Reese]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

If you're running a 64bit kernel with N32 userspace, shmctl will corrupt
memory in userspace. When copy_shmid_to_user() is called, it copies the
entire kernel shmid_ds into userspace. For a 64bit kernel, this is 88
bytes. In N32 userspace it is 76 bytes.

My hack to get around the problem is attached, but I expect someone here
will be able to come up with a better fix. shmid_ds contains a lot of
members that are marked unused. Are these really useless?

Chad


[-- Attachment #2: shared_memory_shmctl_fix.patch --]
[-- Type: text/x-patch, Size: 810 bytes --]

Index: linux/ipc/shm.c
===================================================================
RCS file: /repository/octsw/linux/kernel_2.6/linux/ipc/shm.c,v
retrieving revision 1.1.1.6
retrieving revision 1.2
diff -u -r1.1.1.6 -r1.2
--- linux/ipc/shm.c	7 Jun 2006 19:19:51 -0000	1.1.1.6
+++ linux/ipc/shm.c	22 Jul 2006 02:26:11 -0000	1.2
@@ -321,7 +321,11 @@
 		out.shm_lpid	= in->shm_lpid;
 		out.shm_nattch	= in->shm_nattch;
 
-		return copy_to_user(buf, &out, sizeof(out));
+		/* Use offsetof() instead of sizeof() since N32 userspace has a 
+		    different size including the unused fields. This just copies 
+		    what is used. The old method would corrupt data after the 
+		    structure */
+		return copy_to_user(buf, &out, offsetof(struct shmid_ds, shm_unused2));
 	    }
 	default:
 		return -EINVAL;

Re: 64bit kernel/N32 userspace - shmctl corrupts userspace memory

[Ralf Baechle]

On Tue, Jul 25, 2006 at 05:32:41PM -0700, Chad Reese wrote:

> If you're running a 64bit kernel with N32 userspace, shmctl will corrupt
> memory in userspace. When copy_shmid_to_user() is called, it copies the
> entire kernel shmid_ds into userspace. For a 64bit kernel, this is 88
> bytes. In N32 userspace it is 76 bytes.
> 
> My hack to get around the problem is attached, but I expect someone here
> will be able to come up with a better fix. shmid_ds contains a lot of
> members that are marked unused. Are these really useless?

Can you try below patch?

  Ralf

diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
index 98abbc5..605d393 100644
--- a/arch/mips/kernel/scall64-n32.S
+++ b/arch/mips/kernel/scall64-n32.S
@@ -150,7 +150,7 @@ EXPORT(sysn32_call_table)
 	PTR	sys_madvise
 	PTR	sys_shmget
 	PTR	sys32_shmat
-	PTR	sys_shmctl			/* 6030 */
+	PTR	compat_sys_shmctl		/* 6030 */
 	PTR	sys_dup
 	PTR	sys_dup2
 	PTR	sys_pause

Re: 64bit kernel/N32 userspace - shmctl corrupts userspace memory

[Ralf Baechle]

On Tue, Jul 25, 2006 at 05:32:41PM -0700, Chad Reese wrote:

> If you're running a 64bit kernel with N32 userspace, shmctl will corrupt
> memory in userspace. When copy_shmid_to_user() is called, it copies the
> entire kernel shmid_ds into userspace. For a 64bit kernel, this is 88
> bytes. In N32 userspace it is 76 bytes.
> 
> My hack to get around the problem is attached, but I expect someone here
> will be able to come up with a better fix. shmid_ds contains a lot of
> members that are marked unused. Are these really useless?

They need to be there for compatibility with older revisions of the
structure - we can increase the size of the structure but not decrease.

Talking about compatibility, you change breaks N64 compatibility ...

  Ralf

Re: 64bit kernel/N32 userspace - shmctl corrupts userspace memory

[Chad Reese]

compat.c is only included if CONFIG_SYSVIPC_COMPAT is defined. This
isn't anywhere in 2.6.16.26. Is this what you're refering to?

Chad

Ralf Baechle wrote:
> On Tue, Jul 25, 2006 at 05:32:41PM -0700, Chad Reese wrote:
> 
> 
>>If you're running a 64bit kernel with N32 userspace, shmctl will corrupt
>>memory in userspace. When copy_shmid_to_user() is called, it copies the
>>entire kernel shmid_ds into userspace. For a 64bit kernel, this is 88
>>bytes. In N32 userspace it is 76 bytes.
>>
>>My hack to get around the problem is attached, but I expect someone here
>>will be able to come up with a better fix. shmid_ds contains a lot of
>>members that are marked unused. Are these really useless?
> 
> 
> Can you try below patch?
> 
>   Ralf
> 
> diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
> index 98abbc5..605d393 100644
> --- a/arch/mips/kernel/scall64-n32.S
> +++ b/arch/mips/kernel/scall64-n32.S
> @@ -150,7 +150,7 @@ EXPORT(sysn32_call_table)
>  	PTR	sys_madvise
>  	PTR	sys_shmget
>  	PTR	sys32_shmat
> -	PTR	sys_shmctl			/* 6030 */
> +	PTR	compat_sys_shmctl		/* 6030 */
>  	PTR	sys_dup
>  	PTR	sys_dup2
>  	PTR	sys_pause
> 

-- 

Chad Reese <kreese@caviumnetworks.com>
Cavium Networks
Phone: 650 - 623 - 7038
Cell: 321 - 438 - 7753

Re: 64bit kernel/N32 userspace - shmctl corrupts userspace memory

[Atsushi Nemoto]

On Tue, 25 Jul 2006 19:30:45 -0700, Chad Reese <creese@caviumnetworks.com> wrote:
> compat.c is only included if CONFIG_SYSVIPC_COMPAT is defined. This
> isn't anywhere in 2.6.16.26. Is this what you're refering to?

You can add this to arch/mips/Kconfig:

config SYSVIPC_COMPAT
	bool
	depends on COMPAT && SYSVIPC
	default y

BTW, it seems many SYSVIPC functions in arch/mips/kernel/linux32.c can
be replaced by this common code.

---
Atsushi Nemoto