Re: [Xen-devel] [RFC Patch v4 5/9] check if mfn is supported by IOCTL_PRIVCMD_MMAPBATCH before calling ioctl()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-09-22 at 13:59 +0800, Wen Congyang wrote:
> If mfn is invalid, ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, ..) also returns 0,
> and set error information in error bits(bits28-31). So if the user input
> a large valid mfn, we cannot reliably distinguish between a large MFN and
> an error. So we should check the input mfn before calling ioctl().
> The user can input more than one mfn, and part of them are ~0UL. In this
> case, the user expects we can map the memory for all valid mfn. So we
> cannot just return NULL if some mfn is not supported.

I don't follow this last bit. linux_privcmd_map_foreign_bulk already
returns NULL and maps nothing in some error cases (e.g. mmap failure),
what is the problem also doing that here?

The way you have it here we need to worry about what the behaviour of
Xen/privcmd is on pfn==~0UL, and whether we can rely on it. Far better
to just declare such attempts to be fundamentally broken on the part of
the caller and return.

Ian.

> Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
> ---
>  tools/libxc/xc_linux_osdep.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
> index a19e4b6..d11bcee 100644
> --- a/tools/libxc/xc_linux_osdep.c
> +++ b/tools/libxc/xc_linux_osdep.c
> @@ -321,6 +321,18 @@ static void *linux_privcmd_map_foreign_bulk(xc_interface 
> *xch, xc_osdep_handle h
>          }
>  
>          memcpy(pfn, arr, num * sizeof(*arr));
> +        for ( i = 0; i < num; i++ )
> +        {
> +            /*
> +             * IOCTL_PRIVCMD_MMAPBATCH doesn't support the mfn which
> +             * error bits are set
> +             */
> +            if ( pfn[i] & PRIVCMD_MMAPBATCH_MFN_ERROR )
> +            {
> +                pfn[i] = ~0UL;
> +                err[i] = -EINVAL;
> +            }
> +        }
>  
>          ioctlx.num = num;
>          ioctlx.dom = dom;
> @@ -333,6 +345,9 @@ static void *linux_privcmd_map_foreign_bulk(xc_interface 
> *xch, xc_osdep_handle h
>  
>          for ( i = 0; i < num; ++i )
>          {
> +            if ( pfn[i] == ~0UL )
> +                continue;
> +
>              switch ( pfn[i] ^ arr[i] )
>              {
>              case 0:



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel