IA-32 emulation issues

I ran some IA-32 test suites last week that uncovered a bunch of issues in the IA-32 emulation layer that I wanted to report here.

a) semctl doesn't check for bad cmd

@@ -2166,6 +2166,9 @@
 	else
 		fourth.__pad = (void *)A(pad);
 	switch (third) {
+	      default:
+		err = -EINVAL;
+		break;
 	      case IPC_INFO:
 	      case IPC_RMID:
 	      case IPC_SET:

b) getdents64 - the system call succeeds, but glibc sets EOVERFLOW. We may want to think about getting rid of "struct linux32_dirent" at some point.

History from glibc sources:

/* The getdents64 syscall was introduced in 2.4.0-test7. We test for
2.4.1 for the earliest version we know the syscall is available. */
#if __LINUX_KERNEL_VERSION >= 132097

# define __ASSUME_GETDENTS64_SYSCALL    1
#endif

c) readv and iov_len

Single UNIX spec says that readv should return:

[EINVAL]
The sum of the iov_len values in the iov array overflowed an ssize_t.

The following (untested) patch should fix it. There may be a case for moving this check into userland.

@@ -26,6 +26,7 @@
 #include <linux/uio.h>
 #include <linux/smp_lock.h>
 #include <linux/dnotify.h>
+#include <linux/personality.h>
 
 #include <asm/uaccess.h>
 

@@ -268,7 +269,10 @@
 		   
 		   FIXME: put in a proper limits.h for each platform */
 #if BITS_PER_LONG=64
-		if (tot_len > 0x7FFFFFFFFFFFFFFFUL)
+		if ((current->personality & PER_LINUX32) 
+                    && (tot_len > 0x7FFFFFFFUL))
+		    goto out;
+		else if (tot_len > 0x7FFFFFFFFFFFFFFFUL)
 #else
 		if (tot_len > 0x7FFFFFFFUL)
 #endif		

d) msgctl(id, IPC_STAT, &buf) does't behave as expected

This seems to be related to linux/ipc.h:

#if defined(__ia64__) || defined(__hppa__)
/* On IA-64 and PA-RISC, we always use the "64-bit version" of the IPC structures. */

# define ipc_parse_version(cmd)	IPC_64
#else
int ipc_parse_version (int *cmd);
#endif

However, sys_ia32.c:msgctl32 does a version check against IPC_64 to figure out whether to use struct msqid_ds or msqid64_ds. I think it should always be using msqid64_ds, given the above comment.

-Arun

Arun> I ran some IA-32 test suites last week that uncovered a bunch of issues Arun> in the IA-32 emulation layer that I wanted to report here.

Arun> a) semctl doesn't check for bad cmd

  Arun> --- sys_ia32.c	Wed Jun  5 15:39:54 2002
  Arun> +++ sys_ia32.c.new	Thu Dec 19 17:27:50 2002
  Arun> @@ -2166,6 +2166,9 @@
  Arun> else
  Arun> fourth.__pad = (void *)A(pad);
  Arun> switch (third) {
  Arun> +	      default:
  Arun> +		err = -EINVAL;
  Arun> +		break;
  Arun> case IPC_INFO:
  Arun> case IPC_RMID:
  Arun> case IPC_SET:

I applied this patch to the 2.5 tree.

Arun> b) getdents64 - the system call succeeds, but glibc sets EOVERFLOW. We Arun> may want to think about getting rid of "struct linux32_dirent" at Arun> some point.

Arun> History from glibc sources:

Arun> /* The getdents64 syscall was introduced in 2.4.0-test7. We test for Arun> 2.4.1 for the earliest version we know the syscall is available. */ Arun> #if __LINUX_KERNEL_VERSION >= 132097
Arun> # define __ASSUME_GETDENTS64_SYSCALL 1
Arun> #endif

If you muck with this, it'd probably be best to put it in fs/compat.c at the same time.

Arun> c) readv and iov_len

Arun> Single UNIX spec says that readv should return:

Arun> [EINVAL]
Arun> The sum of the iov_len values in the iov array overflowed an ssize_t.

Arun> The following (untested) patch should fix it. There may be a case for Arun> moving this check into userland.

Arun> --- linux/fs/read_write.c Mon Dec 16 01:06:56 2002
Arun> +++ linux/fs/read_write.c.new Thu Dec 19 16:41:33 2002
Arun> @@ -26,6 +26,7 @@
Arun> #include <linux/uio.h>
Arun> #include <linux/smp_lock.h>
Arun> #include <linux/dnotify.h>
Arun> +#include <linux/personality.h>

Arun> #include <asm/uaccess.h>

Arun> @@ -268,7 +269,10 @@

Arun> FIXME: put in a proper limits.h for each platform */ Arun> #if BITS_PER_LONG=64
Arun> - if (tot_len > 0x7FFFFFFFFFFFFFFFUL)
Arun> + if ((current->personality & PER_LINUX32)
Arun> + && (tot_len > 0x7FFFFFFFUL))
Arun> + goto out;
Arun> + else if (tot_len > 0x7FFFFFFFFFFFFFFFUL)
Arun> #else
Arun> if (tot_len > 0x7FFFFFFFUL)
Arun> #endif

Generic code generally shouldn't be hacked for compatibility support. Instead, we can create a syscall wrapper, like we do for all other cases of similar nature. That way, the native case doesn't get impacted by the compatibility layer.

Arun> d) msgctl(id, IPC_STAT, &buf) does't behave as expected

Arun> This seems to be related to linux/ipc.h:

  Arun> #if defined(__ia64__) || defined(__hppa__)
  Arun> /* On IA-64 and PA-RISC, we always use the "64-bit version" of the IPC structures.  */
  Arun> # define ipc_parse_version(cmd)	IPC_64
  Arun> #else
  Arun> int ipc_parse_version (int *cmd);
  Arun> #endif

Arun> However, sys_ia32.c:msgctl32 does a version check against IPC_64 to Arun> figure out whether to use struct msqid_ds or msqid64_ds. I think it Arun> should always be using msqid64_ds, given the above comment.

Yes, I suspect some more translation is required in the syscall-compatibility layer.

Thanks,

	--david

I've done some more debugging on this and I think there is no problem with getdents/getdents64.

The problem was with a test case that did pwrite(2) with offset = -1 and it was expecting the test to fail. However, on IA-64, the test case passed, leaving this temp file on the disk:

$ ls -l /tmp/pwrbcneW5/pwrite_file 
-rw-r--r--    1 tester tester 4294968319 Jan  2 15:34 /tmp/pwrbcneW5/pwrite_file

Subsequently, the test harness tries to cleanup the file and does a lstat on it, which results in EOVERFLOW.

I'll follow up separately on the other two issues I raised.

-Arun

Arun> I've done some more debugging on this and I think there is no problem Arun> with getdents/getdents64.

Arun> The problem was with a test case that did pwrite(2) with offset = -1 Arun> and it was expecting the test to fail. However, on IA-64, the test Arun> case passed, leaving this temp file on the disk:

Arun> $ ls -l /tmp/pwrbcneW5/pwrite_file
Arun> -rw-r--r-- 1 tester tester 4294968319 Jan 2 15:34 /tmp/pwrbcneW5/pwrite_file

Arun> Subsequently, the test harness tries to cleanup the file and does a Arun> lstat on it, which results in EOVERFLOW.

I think this has been fixed in 2.5 already: the problem was that the x86 version of sys_open() ended up turning on the O_LARGEFILE flag, which was wrong (I think Andi Kleen pointed this out, originally). The fix was to add the sys32_open() wrapper. I don't recall if this has been added to 2.4.xx already. If not, it probably ought to be added.

	--david

I copied this change from 2.5 into 2.4:

# 02/12/03 [email protected] 1.786.149.17
# ia64: For ia32 emulation, do not turn on O_LARGEFILE automatically
# on open(). Reported by Andi Kleen.