[Linux-ia64] IA-32 emulation issues

[Linux-ia64] IA-32 emulation issues

[Arun Sharma]

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

--- sys_ia32.c	Wed Jun  5 15:39:54 2002
+++ sys_ia32.c.new	Thu Dec 19 17:27:50 2002
@@ -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.

--- linux/fs/read_write.c	Mon Dec 16 01:06:56 2002
+++ linux/fs/read_write.c.new	Thu Dec 19 16:41:33 2002
@@ -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

Re: [Linux-ia64] IA-32 emulation issues

[David Mosberger]

>>>>> On Thu, 26 Dec 2002 13:27:29 -0800, Arun Sharma <arun.sharma@intel.com> said:

  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

Re: [Linux-ia64] IA-32 emulation issues

[Arun Sharma]

David Mosberger <davidm@napali.hpl.hp.com> writes:
> 
>   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.
> 

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

Re: [Linux-ia64] IA-32 emulation issues

[David Mosberger]

>>>>> On 02 Jan 2003 15:31:04 -0800, Arun Sharma <arun.sharma@intel.com> said:

  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

Re: [Linux-ia64] IA-32 emulation issues

[Bjorn Helgaas]

> 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.

I copied this change from 2.5 into 2.4:

# 02/12/03      davidm@tiger.hpl.hp.com 1.786.149.17
# ia64: For ia32 emulation, do not turn on O_LARGEFILE automatically
#       on open().  Reported by Andi Kleen.