On Jul 27, 2006, at 2:30 PM, Hollis Blanchard wrote:

On Wed, 2006-07-26 at 18:40 +0000, Xen patchbot-xenppc-unstable wrote:
diff -r 518e13229929 -r 50ef9c9c717c xen/arch/powerpc/usercopy.c
[snip]
+    return (desc->address[0] == XENCOMM_INVALID);

You can see how the function just above, xencomm_add_offset(),
overwrites the addresses with XENCOMM_INVALID. I think this incremental
patch is needed:

diff -r 42213c6e1928 xen/arch/powerpc/usercopy.c
[snip]
+    for (i = 0; i < desc->nr_addrs; i++)
+        if (desc->address[i] != XENCOMM_INVALID)
+            return 0;
+    return 1;
+}

In other words, if there are any valid addresses at all, the handle is
not null. Comments? Does this code work for you Jimi?

The code above is necessary to catch the case where the Linux function:
  drivers/xen/core/xencomm.c <global> 56 static int __xencomm_init()

is passed a 'buffer' param that is NULL.

I was going to perform the solution below but I thought checking address[0] was a good shortcut but as you point out, xencomm_handle_is_null() could return a false positive.

I think the correct solution (pretty sure it would cover it all) would be to arrange for __xencomm_init(buffer=NULL) to set desc- >nr_addrs = 0, never touching desc->address[] and checking for (nr_addrs==0) in xencomm_handle_is_null(), also reducing the number of checks and assignments required to communicate "nothing"

the following should be sufficient (sorry the diff got mangled):

diff -r e30fba67c38d drivers/xen/core/xencomm.c
--- a/drivers/xen/core/xencomm.c        Wed Jul 26 15:00:06 2006 -0400
+++ b/drivers/xen/core/xencomm.c        Thu Jul 27 16:24:00 2006 -0400
@@ -86,11 +86,13 @@ static int __xencomm_init(struct xencomm
                               __func__, recorded, bytes);
                        return -ENOSPC;
        }
+
+                       /* mark remaining addresses invalid (just for safety) */
+                       while (i < desc->nr_addrs)
+                                desc->address[i++] = XENCOMM_INVALID;
+               } else {
+                               desc->nr_addrs = 0;
        }
-
-               /* mark remaining addresses invalid (just for safety) */
-               while (i < desc->nr_addrs)
-                       desc->address[i++] = XENCOMM_INVALID;
        desc->magic = XENCOMM_MAGIC;

diff -r f77ff6eb8d22 xen/arch/powerpc/usercopy.c
--- a/xen/arch/powerpc/usercopy.c       Wed Jul 26 14:27:51 2006 -0400
+++ b/xen/arch/powerpc/usercopy.c       Thu Jul 27 16:26:10 2006 -0400
@@ -238,5 +238,5 @@ int xencomm_handle_is_null(void *ptr)
     desc = (struct xencomm_desc *)paddr_to_maddr((unsigned long)ptr);
-    return (desc->address[0] == XENCOMM_INVALID);
-}
+    return (desc->nr_addrs == 0);
+}


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel

Reply via email to