[Qemu-devel] [PATCH] zero/NULL fixes.

[Qemu-devel] [PATCH] zero/NULL fixes.

[Thayne Harbaugh]

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

The EFAULT changes use a result of NULL to detect a failure from lock*()
functions.  There are syscalls that accept NULL as a valid argument and
now the syscalls return -EFAULT.  These patches allow appropriate
syscalls to accept NULL.

I have put together a regression test harness wrapped around the Linux
Test Project (LTP).  I've been able to find regressions that were caused
by the EFAULT changes.  It's more exhaustive than running an ls
executable and has helped find existing bugs as well as regressions.  It
will run regression tests for multiple architectures.

I'll be sending a few more patches for this same type of regression.  I
should also be sending in the test harness once I've worked out a few
more details.

[-- Attachment #2: 19_zero_null.1.getgroups.patch --]
[-- Type: text/x-patch, Size: 1023 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-28 16:36:53.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-28 17:03:31.000000000 -0700
@@ -5399,7 +5399,7 @@
 
             grouplist = alloca(gidsetsize * sizeof(gid_t));
             ret = get_errno(getgroups(gidsetsize, grouplist));
-            if (!is_error(ret)) {
+            if (!is_error(ret) && arg1) {
                 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 0);
                 if (!target_grouplist)
                     goto efault;
@@ -5549,7 +5549,7 @@
 
             grouplist = alloca(gidsetsize * sizeof(gid_t));
             ret = get_errno(getgroups(gidsetsize, grouplist));
-            if (!is_error(ret)) {
+            if (!is_error(ret) && arg1) {
                 target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 4, 0);
                 if (!target_grouplist) {
                     ret = -TARGET_EFAULT;

[-- Attachment #3: 19_zero_null.2.read.patch --]
[-- Type: text/x-patch, Size: 867 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-28 17:26:46.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-28 17:29:11.000000000 -0700
@@ -3398,10 +3398,14 @@
         ret = 0; /* avoid warning */
         break;
     case TARGET_NR_read:
-        if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
-            goto efault;
-        ret = get_errno(read(arg1, p, arg3));
-        unlock_user(p, arg2, ret);
+        if (!arg3)
+            ret = 0;
+        else {
+            if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
+                goto efault;
+            ret = get_errno(read(arg1, p, arg3));
+            unlock_user(p, arg2, ret);
+        }
         break;
     case TARGET_NR_write:
         if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))

[-- Attachment #4: 19_zero_null.3.accept.patch --]
[-- Type: text/x-patch, Size: 431 bytes --]

Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-28 22:44:27.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-28 22:46:16.000000000 -0700
@@ -1203,7 +1203,7 @@
     abi_long ret;
 
     if (get_user_u32(addrlen, target_addrlen_addr))
-        return -TARGET_EFAULT;
+        return -TARGET_EINVAL;
 
     addr = alloca(addrlen);
 

[-- Attachment #5: 19_zero_null.4.acct.patch --]
[-- Type: text/x-patch, Size: 1147 bytes --]

Index: qemu/linux-user/path.c
===================================================================
--- qemu.orig/linux-user/path.c	2007-11-28 23:13:12.000000000 -0700
+++ qemu/linux-user/path.c	2007-11-28 23:13:24.000000000 -0700
@@ -152,7 +152,7 @@
 {
     /* Only do absolute paths: quick and dirty, but should mostly be OK.
        Could do relative by tracking cwd. */
-    if (!base || name[0] != '/')
+    if (!base || !name || name[0] != '/')
 	return name;
 
     return follow_path(base, name) ?: name;
Index: qemu/linux-user/syscall.c
===================================================================
--- qemu.orig/linux-user/syscall.c	2007-11-28 23:01:10.000000000 -0700
+++ qemu/linux-user/syscall.c	2007-11-28 23:13:06.000000000 -0700
@@ -3864,8 +3864,12 @@
         goto unimplemented;
 #endif
     case TARGET_NR_acct:
-        if (!(p = lock_user_string(arg1)))
-            goto efault;
+        if (arg1) {
+            if (!(p = lock_user_string(arg1)))
+                goto efault;
+        } else {
+            p = NULL;
+        }
         ret = get_errno(acct(path(p)));
         unlock_user(p, arg1, 0);
         break;

Re: [Qemu-devel] [PATCH] zero/NULL fixes.

[Lauro Ramos Venancio]

I think these patches are important bugfixes and they should be
applied on the qemu head.
They fix some LTP tests.

Lauro

2007/12/11, Thayne Harbaugh <thayne@c2.net>:
> The EFAULT changes use a result of NULL to detect a failure from lock*()
> functions.  There are syscalls that accept NULL as a valid argument and
> now the syscalls return -EFAULT.  These patches allow appropriate
> syscalls to accept NULL.
>
> I have put together a regression test harness wrapped around the Linux
> Test Project (LTP).  I've been able to find regressions that were caused
> by the EFAULT changes.  It's more exhaustive than running an ls
> executable and has helped find existing bugs as well as regressions.  It
> will run regression tests for multiple architectures.
>
> I'll be sending a few more patches for this same type of regression.  I
> should also be sending in the test harness once I've worked out a few
> more details.
>
>

Re: [Qemu-devel] [PATCH] zero/NULL fixes.

[Thayne Harbaugh]

On Wed, 2008-01-09 at 16:14 -0300, Lauro Ramos Venancio wrote:
> I think these patches are important bugfixes and they should be
> applied on the qemu head.
> They fix some LTP tests.

Tonight I'll be pulling my local tree up to CVS current and I'll resend
any remaining EFAULT and zero/NULL patches that haven't been included.
Right now I simply haven't tracked which patches have been incorporated
and which ones are pending.

Thanks for the interest.

> 2007/12/11, Thayne Harbaugh <thayne@c2.net>:
> > The EFAULT changes use a result of NULL to detect a failure from lock*()
> > functions.  There are syscalls that accept NULL as a valid argument and
> > now the syscalls return -EFAULT.  These patches allow appropriate
> > syscalls to accept NULL.
> >
> > I have put together a regression test harness wrapped around the Linux
> > Test Project (LTP).  I've been able to find regressions that were caused
> > by the EFAULT changes.  It's more exhaustive than running an ls
> > executable and has helped find existing bugs as well as regressions.  It
> > will run regression tests for multiple architectures.
> >
> > I'll be sending a few more patches for this same type of regression.  I
> > should also be sending in the test harness once I've worked out a few
> > more details.
> >
> >