WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] linux/privcmd: fix for proper operation in compa

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] linux/privcmd: fix for proper operation in compat mode
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Wed, 6 Jan 2010 13:27:18 +1100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 05 Jan 2010 18:27:48 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4B4344040200007800028412@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4B4344040200007800028412@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
On Tue, Jan 05, 2010 at 12:52:04PM +0000, Jan Beulich wrote:
> - sizeof(struct privcmd_mmapbatch_32) was wrong
> - MFN array must be translated for IOCTL_PRIVCMD_MMAPBATCH
> 
> Also, the error indicator of IOCTL_PRIVCMD_MMAPBATCH should be in the
> top nibble (it is documented that way in include/xen/public/privcmd.h
> and include/xen/compat_ioctl.h), but since that is an incompatible
> change it is not being done here (instead, a new ioctl with proper
> behavior will need to be added).
> 
> As usual, written against 2.6.32.2 and made apply to the 2.6.18
> tree without further testing.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> --- head-2010-01-04.orig/drivers/xen/privcmd/compat_privcmd.c 2009-11-06 
> 10:45:48.000000000 +0100
> +++ head-2010-01-04/drivers/xen/privcmd/compat_privcmd.c      2010-01-04 
> 13:50:00.000000000 +0100
> @@ -51,17 +51,49 @@ int privcmd_ioctl_32(int fd, unsigned in
>               struct privcmd_mmapbatch *p;
>               struct privcmd_mmapbatch_32 *p32;
>               struct privcmd_mmapbatch_32 n32;
> +#ifdef xen_pfn32_t
> +             xen_pfn_t *__user arr;
> +             xen_pfn32_t *__user arr32;
> +             unsigned int i;
> +#endif
>  
>               p32 = compat_ptr(arg);
>               p = compat_alloc_user_space(sizeof(*p));
>               if (copy_from_user(&n32, p32, sizeof(n32)) ||
>                   put_user(n32.num, &p->num) ||
>                   put_user(n32.dom, &p->dom) ||
> -                 put_user(n32.addr, &p->addr) ||
> -                 put_user(compat_ptr(n32.arr), &p->arr))
> +                 put_user(n32.addr, &p->addr))
>                       return -EFAULT;
> +#ifdef xen_pfn32_t
> +             arr = compat_alloc_user_space(n32.num * sizeof(*arr)
> +                                           + sizeof(*p));
> +             arr32 = compat_ptr(n32.arr);
> +             for (i = 0; i < n32.num; ++i) {
> +                     xen_pfn32_t mfn;
> +
> +                     if (get_user(mfn, arr32 + i) || put_user(mfn, arr + i))
> +                             return -EFAULT;
> +             }
> +
> +             if (put_user(arr, &p->arr))
> +                     return -EFAULT;
> +#else
> +             if (put_user(compat_ptr(n32.arr), &p->arr))
> +                     return -EFAULT;
> +#endif
>               
>               ret = sys_ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, (unsigned long)p);
> +
> +#ifdef xen_pfn32_t
> +             for (i = 0; !ret && i < n32.num; ++i) {
> +                     xen_pfn_t mfn;
> +
> +                     if (get_user(mfn, arr + i) || put_user(mfn, arr32 + i))
> +                             ret = -EFAULT;
> +                     else if (mfn != (xen_pfn32_t)mfn)
> +                             ret = -ERANGE;
> +             }
> +#endif
>       }
>               break;
>       default:

Perhaps this could be refactored to reduce or remove the #ifdefs ?

> --- head-2010-01-04.orig/include/xen/compat_ioctl.h   2007-07-10 
> 09:42:30.000000000 +0200
> +++ head-2010-01-04/include/xen/compat_ioctl.h        2009-12-17 
> 15:40:40.000000000 +0100
> @@ -23,6 +23,11 @@
>  #define __LINUX_XEN_COMPAT_H__ 
>  
>  #include <linux/compat.h>
> +#include <linux/compiler.h>
> +
> +#if defined(CONFIG_X86) || defined(CONFIG_IA64)
> +#define xen_pfn32_t __u32
> +#endif

Is it usual in xen for a type to be a #define in xen?

Ito my mind it would make things a lot clearer to use a #define called
SOME_FEATURE and then either use __u32 directly for the type or
typedef __u32 xen_pfn32_t.

>  
>  extern int privcmd_ioctl_32(int fd, unsigned int cmd, unsigned long arg);
>  struct privcmd_mmap_32 {
> @@ -34,7 +39,14 @@ struct privcmd_mmap_32 {
>  struct privcmd_mmapbatch_32 {
>       int num;     /* number of pages to populate */
>       domid_t dom; /* target domain */
> +#if defined(CONFIG_X86) || defined(CONFIG_IA64)
> +     union {      /* virtual address */
> +             __u64 addr __packed;
> +             __u32 va;
> +     };
> +#else
>       __u64 addr;  /* virtual address */
> +#endif
>       compat_uptr_t arr; /* array of mfns - top nibble set on err */
>  };
>  #define IOCTL_PRIVCMD_MMAP_32                   \

Its unclear to me why va is necessary.
It doesn't seem to be used anywhere in the patch.


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

<Prev in Thread] Current Thread [Next in Thread>