Skip to contentSkip to conversation

MIPS: o32: Get rid of useless wrapper for llseek

PATCH In review Ralf Baechle 4 participants 4 messages Mar 25, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 2/2 MIPS: o32: Get rid of useless wrapper for llseek

 arch/mips/kernel/linux32.c     |    7 -------
 arch/mips/kernel/scall64-o32.S |    2 +-
 2 files changed, 1 insertions(+), 8 deletions(-)
arch/mips/kernel/linux32.c +0 -7
index 49aac6e..ab2da41 100644
@@ -133,13 +133,6 @@ SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy,
 	return sys_ftruncate(fd, merge_64(a2, a3));
 }
 
-SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
-	unsigned long, offset_low, loff_t __user *, result,
-	unsigned long, origin)
-{
-	return sys_llseek(fd, offset_high, offset_low, result, origin);
-}
-
 /* From the Single Unix Spec: pread & pwrite act like lseek to pos + op +
    lseek back to original location.  They fail just like lseek does on
    non-seekable files.  */
arch/mips/kernel/scall64-o32.S +1 -1
index b0fef4f..d928614 100644
@@ -343,7 +343,7 @@ sys_call_table:
 	PTR	sys_ni_syscall	 		/* for afs_syscall */
 	PTR	sys_setfsuid
 	PTR	sys_setfsgid
-	PTR	sys_32_llseek			/* 4140 */
+	PTR	sys_llseek			/* 4140 */
 	PTR	compat_sys_getdents
 	PTR	compat_sys_select
 	PTR	sys_flock
#1 lore raw
Quoted 11 lines · On Wed, 25 Mar 2009 01:15:55 +0100 Ralf Baechle <[email protected]> wrote:
 arch/mips/kernel/linux32.c     |    7 -------
 arch/mips/kernel/scall64-o32.S |    2 +-
 2 files changed, 1 insertions(+), 8 deletions(-)
arch/mips/kernel/linux32.c +0 -0
index 49aac6e..ab2da41 100644

arch/mips/kernel/linux32.c SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy, · line 142

Quoted 10 lines
arch/mips/kernel/linux32.c +0 -7
@@ -133,13 +133,6 @@ SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy,
 	return sys_ftruncate(fd, merge_64(a2, a3));
 }
 
-SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
-	unsigned long, offset_low, loff_t __user *, result,
-	unsigned long, origin)
-{
-	return sys_llseek(fd, offset_high, offset_low, result, origin);
-}
-

Ah.. this hunk seems to be the origin of the bug. git commit dbda6ac0897603f6c6dfadbbc37f9882177ec7ac "MIPS: CVE-2009-0029: Enable syscall wrappers." contains this:

-asmlinkage int sys32_llseek(unsigned int fd, unsigned int offset_high,
- unsigned int offset_low, loff_t __user * result,
- unsigned int origin)
+SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
+ unsigned long, offset_low, loff_t __user *, result,
+ unsigned long, origin)

Here you converted offset_low from unsigned int to unsigned long. Hence you lost the clearing of the upper 32 bits. That would explain the bug. Any chance the process where the bug was seen was a compat process?

Patch below would probably fix it.

Btw. there are a lot of int->long conversions in the git commit mentioned above. AFAICS all(?) of the conversions are wrong.

arch/mips/kernel/linux32.c +3 -3
index 1a86f84..5abcc7f 100644
@@ -134,9 +134,9 @@ SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy,
 	return sys_ftruncate(fd, merge_64(a2, a3));
 }
 
-SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
-	unsigned long, offset_low, loff_t __user *, result,
-	unsigned long, origin)
+SYSCALL_DEFINE5(32_llseek, unsigned int, fd, unsigned int, offset_high,
+		unsigned int, offset_low, loff_t __user *, result,
+		unsigned int, origin)
 {
 	return sys_llseek(fd, offset_high, offset_low, result, origin);
 }
#2 lore raw
Quoted 36 lines · Heiko Carstens
Quoted 11 lines · On Wed, 25 Mar 2009 01:15:55 +0100 Ralf Baechle <[email protected]> wrote:
 arch/mips/kernel/linux32.c     |    7 -------
 arch/mips/kernel/scall64-o32.S |    2 +-
 2 files changed, 1 insertions(+), 8 deletions(-)
arch/mips/kernel/linux32.c +0 -0
index 49aac6e..ab2da41 100644

arch/mips/kernel/linux32.c SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy, · line 142

Quoted 10 lines
arch/mips/kernel/linux32.c +0 -7
@@ -133,13 +133,6 @@ SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy,
 	return sys_ftruncate(fd, merge_64(a2, a3));
 }
 
-SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
-	unsigned long, offset_low, loff_t __user *, result,
-	unsigned long, origin)
-{
-	return sys_llseek(fd, offset_high, offset_low, result, origin);
-}
-

Ah.. this hunk seems to be the origin of the bug. git commit dbda6ac0897603f6c6dfadbbc37f9882177ec7ac "MIPS: CVE-2009-0029: Enable syscall wrappers." contains this:

(quoted diff) +3 -3
-asmlinkage int sys32_llseek(unsigned int fd, unsigned int offset_high,
-			    unsigned int offset_low, loff_t __user * result,
-			    unsigned int origin)
+SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
+	unsigned long, offset_low, loff_t __user *, result,
+	unsigned long, origin)
Quoted 3 lines

Here you converted offset_low from unsigned int to unsigned long. Hence you lost the clearing of the upper 32 bits. That would explain the bug. Any chance the process where the bug was seen was a compat process?

$ file /sbin/e2fsck 
/sbin/e2fsck: ELF 32-bit MSB executable, MIPS, MIPS-I version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.8, with unknown capability 0x41000000 = 0xf676e75, stripped
Quoted 1 line

Patch below would probably fix it.

This patch works for me on the aforementioned Debian system.

Quoted 21 lines

Btw. there are a lot of int->long conversions in the git commit mentioned above. AFAICS all(?) of the conversions are wrong.

arch/mips/kernel/linux32.c +3 -3
index 1a86f84..5abcc7f 100644
@@ -134,9 +134,9 @@ SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy,
 	return sys_ftruncate(fd, merge_64(a2, a3));
 }
 
-SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
-	unsigned long, offset_low, loff_t __user *, result,
-	unsigned long, origin)
+SYSCALL_DEFINE5(32_llseek, unsigned int, fd, unsigned int, offset_high,
+		unsigned int, offset_low, loff_t __user *, result,
+		unsigned int, origin)
 {
 	return sys_llseek(fd, offset_high, offset_low, result, origin);
 }
Signature · 1 line
dann frazier
Tested-by on v1 2/2
#3 lore raw
In reply to Ralf Baechle
Quoted 1 line · Ralf Baechle
Quoted 36 lines
 arch/mips/kernel/linux32.c     |    7 -------
 arch/mips/kernel/scall64-o32.S |    2 +-
 2 files changed, 1 insertions(+), 8 deletions(-)
arch/mips/kernel/linux32.c +0 -7
index 49aac6e..ab2da41 100644
@@ -133,13 +133,6 @@ SYSCALL_DEFINE4(32_ftruncate64, unsigned long, fd, unsigned long, __dummy,
 	return sys_ftruncate(fd, merge_64(a2, a3));
 }
 
-SYSCALL_DEFINE5(32_llseek, unsigned long, fd, unsigned long, offset_high,
-	unsigned long, offset_low, loff_t __user *, result,
-	unsigned long, origin)
-{
-	return sys_llseek(fd, offset_high, offset_low, result, origin);
-}
-
 /* From the Single Unix Spec: pread & pwrite act like lseek to pos + op +
    lseek back to original location.  They fail just like lseek does on
    non-seekable files.  */
arch/mips/kernel/scall64-o32.S +1 -1
index b0fef4f..d928614 100644
@@ -343,7 +343,7 @@ sys_call_table:
 	PTR	sys_ni_syscall	 		/* for afs_syscall */
 	PTR	sys_setfsuid
 	PTR	sys_setfsgid
-	PTR	sys_32_llseek			/* 4140 */
+	PTR	sys_llseek			/* 4140 */
 	PTR	compat_sys_getdents
 	PTR	compat_sys_select
 	PTR	sys_flock
Signature · 1 line
dann frazier