access_ok and CONFIG_MIPS32 for 2.6

Hi,

the mask used in access_ok to check the validity of an address range evaluates to -TASK_SIZE for user processes. In case of CONFIG_MIPS32, TASK_SIZE is defined as 0x7fff8000UL, so -TASK_SIZE evaluates to 0x80008000, making access_ok return false for all addresses with bit 15 and 31 set. Surely the mask should be 0x80000000.

Does anybody know why TASK_SIZE is set to 0x7fff8000 and not 0x80000000 ?

Dimitri

Dimitri Torfs             |  NSCE 
[email protected] |  Sint Stevens Woluwestraat 55
tel: +32 2 2908451        |  1130 Brussel
fax: +32 2 7262686        |  Belgium

There is a weird special case were 32-bit code running on a 64-bit kernel with c0_status.ux set will behave differently than on a 32-bit processor or with c0_status.ux clear. The workaround for 64-bit kernels is to leave the top 32kB of the 2GB user virtual address space unused. For sake of symmetry we do this on both 32-bit and 64-bit kernels.

Ralf

ralf> There is a weird special case were 32-bit code running on a ralf> 64-bit kernel with c0_status.ux set will behave differently than ralf> on a 32-bit processor or with c0_status.ux clear. The ralf> workaround for 64-bit kernels is to leave the top 32kB of the ralf> 2GB user virtual address space unused. For sake of symmetry we ralf> do this on both 32-bit and 64-bit kernels.

Then, access_ok in 2.6 tree is broken, isn't it?

2.4 mips:
#define TASK_SIZE 0x7fff8000UL
#define USER_DS ((mm_segment_t) { (unsigned long) -1L })

2.4 mips64:
#define TASK_SIZE32 0x7fff8000UL
#define TASK_SIZE 0x10000000000UL
#define USER_DS ((mm_segment_t) { -TASK_SIZE })

2.6:
#ifdef CONFIG_MIPS32
#define TASK_SIZE 0x7fff8000UL
#else
#define TASK_SIZE32 0x7fff8000UL
#define TASK_SIZE 0x10000000000UL
#endif
#define USER_DS ((mm_segment_t) { -TASK_SIZE })

It seems there should be another definition of USER_DS for CONFIG_MIPS32 in 2.6.

BTW, there are another problems in uaccess.h as I wrote in
http://www.linux-mips.org/archives/linux-mips/2003-09/msg00035.html.

First, 2.4 mips64 __ua_size is broken.

2.4 mips:

#define __ua_size(size)							\
	(__builtin_constant_p(size) && (signed long) (size) > 0 ? 0 : (size))
2.4 mips64:
#define __ua_size(size)							\
	((__builtin_constant_p(size) && (size)) > 0 ? 0 : (size))
2.6:
#define __ua_size(size)							\
	((__builtin_constant_p(size) && (signed long) (size) > 0) ? 0 : (size))

2.4 mips and 2.6 are identical except for parenthesis.

Second, __access_ok for 64bit kernel is broken both 2.4 and 2.6. It returns 0 if 'addr' + 'size' == TASK_SIZE (which should be OK).

2.4 mips64:

#define __access_ok(addr, size, mask)					\
	(((mask) & ((addr) | ((addr) + (size)) | __ua_size(size))) == 0)
2.6:
#define __access_ok(addr, size, mask)					\
	(((signed long)((mask) & ((addr) | ((addr) + (size)) | __ua_size(size)))) == 0)

I think these macros should be:

2.4 mips64:

#define __access_ok(addr, size, mask)					\
	(((mask) & ((addr) | ((addr) + (size) - 1) | __ua_size(size))) == 0)
2.6:
#define __access_ok(addr, size, mask)					\
	(((signed long)((mask) & ((addr) | ((addr) + (size) - 1) | __ua_size(size)))) == 0)

---
Atsushi Nemoto

Yes, I'm setting USER_DS to 0x80000000 for CONFIG_MIPS32:

@@ -42,7 +42,12 @@
 #endif /* CONFIG_MIPS64 */
 
 #define KERNEL_DS	((mm_segment_t) { 0UL })
+
+#ifdef CONFIG_MIPS32
+#define USER_DS		((mm_segment_t) { 0x80000000UL })
+#else
 #define USER_DS		((mm_segment_t) { -TASK_SIZE })
+#endif
 
 #define VERIFY_READ    0
 #define VERIFY_WRITE   1

The USER_DS mask is also used in scall32-o32.S, scall64-64.S and scall64-032.S. It think it would be cleaner if we replace there also the ">= 0" check with the "== 0" check and add the correct size as you suggested:

@@ -119,10 +119,10 @@
 	bnez	v0, bad_alignment
 
 	LONG_L	v1, TI_ADDR_LIMIT($28)		# in legal address range?
-	LONG_ADDIU	a0, a1, 4
+	LONG_ADDIU	a0, a1, 3 
 	or	a0, a0, a1
 	and	a0, a0, v1
-	bltz	a0, bad_address
+	bnez	a0, bad_address
 
 #ifdef CONFIG_CPU_HAS_LLSC
 	/* Ok, this is the ll/sc case.  World is sane :-)  */

@@ -187,10 +187,10 @@
 	bnez	v0, bad_alignment
 
 	lw	v1, TI_ADDR_LIMIT($28)		# in legal address range?
-	addiu	a0, a1, 4
+	addiu	a0, a1, 3 
 	or	a0, a0, a1
 	and	a0, a0, v1
-	bltz	a0, bad_address
+	bnez	a0, bad_address
 
 #ifdef CONFIG_CPU_HAS_LLSC
 	/* Ok, this is the ll/sc case.  World is sane :-)  */

@@ -280,11 +280,11 @@
 	bnez	v0, sigsegv
 
 	addu	v0, t0, 16			# v0 = usp + 16
-	addu	t1, v0, 12			# 3 32-bit arguments
+	addu	t1, v0, 11			# 3 32-bit arguments
 	lw	v1, TI_ADDR_LIMIT($28)
 	or	v0, v0, t1
 	and	v1, v1, v0
-	bltz	v1, efault
+	bnez	v1, efault
 
 	move	a0, a1				# shift argument registers
 	move	a1, a2

@@ -119,10 +119,10 @@
 	bnez	v0, bad_alignment
 
 	LONG_L	v1, TI_ADDR_LIMIT($28)		# in legal address range?
-	LONG_ADDIU	a0, a1, 4
+	LONG_ADDIU	a0, a1, 3 
 	or	a0, a0, a1
 	and	a0, a0, v1
-	bltz	a0, bad_address
+	bnez	a0, bad_address
 
 #ifdef CONFIG_CPU_HAS_LLSC
 	/* Ok, this is the ll/sc case.  World is sane :-)  */

Dimitri

dimitri> Yes, I'm setting USER_DS to 0x80000000 for CONFIG_MIPS32:

Now we can see this fix in CVS 2.6 tree (Thank you, Ralf).

Then, how about this one?

For example, currently, access_ok(0xfffffff000UL, 0x1000) will return
0. This must be legal (and this is a real problem for n64 mount syscall which may grab user stack. See copy_mount_option()).

---
Atsushi Nemoto