Re: [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file

[Ian Campbell <Ian.Campbell@eu.citrix.com>]

On Tue, 2011-01-25 at 18:10 +0000, Ian Jackson wrote:
> Kamala Narasimhan writes ("Re: [Xen-devel] [PATCH] xl: Perform minimal validation of virtual disk file while parsing config file"):
> > Ian - Apologies for the delay.  I think I have covered all comments so far.  If there are more I will get to it ASAP.  Please let me know.
> 
> Thanks, I have applied your patch, with a minor tweak to make it work
> with Stefano's PHYSTYPE_EMPTY patch.

With this patch applied I get an error using an LVM device via file://
(which is currently necessary on upstream dom0 support in order to
activate the qdisk support):
        # xl cr -c /etc/xen/debian-x86_32p-1
        Parsing config file /etc/xen/debian-x86_32p-1
        libxl: error: libxl.c:854:validate_virtual_disk Virtual disk /dev/VG/debian-x86_32-1 size is 0!
        
My disk stanza is
        # grep ^disk /etc/xen/debian-x86_32p-1
        disk = ['file:/dev/VG/debian-x86_32-1,xvda,w']

I think the issue is in this fragment:
    if ( disk_type == PHYSTYPE_PHY ) {
        if ( !(S_ISBLK(stat_buf.st_mode)) ) {
            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s is not a block device!\n",
                file_name);
            return ERROR_INVAL;
        }
    } else if ( stat_buf.st_size == 0 ) {
        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
        return ERROR_INVAL;
    }
since stat(2) says that st_size is only valid for a regular file or
symbolic link (the latter being irrelevant in this case since we are
using stat() and not lstat()).

Ian.

8<--------------------


# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1296037362 0
# Node ID e4e69622dc95037eab6740f79ecf9c1d05bca529
# Parent  751232406cedb808149ece09480f7a7ec552483d
libxl: only check size of regular files when validating a virtual disk

st_size is only valid for regular files and not block devices.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 751232406ced -r e4e69622dc95 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Jan 26 09:11:34 2011 +0000
+++ b/tools/libxl/libxl.c	Wed Jan 26 10:22:42 2011 +0000
@@ -850,7 +850,7 @@ static int validate_virtual_disk(libxl_c
                 file_name);
             return ERROR_INVAL;
         }
-    } else if ( stat_buf.st_size == 0 ) {
+    } else if ( S_ISREG(stat_buf.st_mode) && stat_buf.st_size == 0 ) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Virtual disk %s size is 0!\n", file_name);
         return ERROR_INVAL;
     }