Re: [Xen-devel] [PATCH V2] libxl: relax readonly check introduced by XSA-142 fix

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 12 Nov 2015, Jim Fehlig wrote:
> The fix for XSA-142 is quite a big hammer, rejecting readonly
> disk configuration even when the requested backend is known to
> support readonly. While it is true that qemu doesn't support
> readonly for emulated IDE or AHCI disks
> 
> $ /usr/lib/xen/bin/qemu-system-i386 \
>  -drive file=/tmp/disk.raw,if=ide,media=disk,format=raw,readonly=on
> qemu-system-i386: Can't use a read-only drive
> 
> $ /usr/lib/xen/bin/qemu-system-i386 -device ahci,id=ahci0 \
>  -drive file=/tmp/disk.raw,if=none,id=ahcidisk-0,format=raw,readonly=on \
>  -device ide-hd,bus=ahci0.0,unit=0,drive=ahcidisk-0
> qemu-system-i386: -device ide-hd,bus=ahci0.0,unit=0,drive=ahcidisk-0:
> Can't use a read-only drive
> 
> It does support readonly SCSI disks
> 
> $ /usr/lib/xen/bin/qemu-system-i386 \
>  -drive file=/tmp/disk.raw,if=scsi,media=disk,format=raw,readonly=on
> [ok]
> 
> Inside a guest using such a disk, the SCSI kernel driver sees write
> protect on
> 
> [   7.339232] sd 2:0:1:0: [sdb] Write Protect is on
> 
> Also, PV drivers support readonly, but the patch rejects such
> configuration even when PV drivers (vdev=xvd*) have been explicitly
> specified and creation of an emulated twin is skiped.
> 
> This follow-up patch loosens the restriction to reject readonly when
> creating and emulated IDE or AHCI disk, but allows it when the backend
> is known to support readonly.
> 
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>



> V2: Along with IDE+readonly, blacklist AHCI+readonly since it is not
>     supported by qemu either.
> 
>  tools/libxl/libxl_dm.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 9c9eaa3..cb6deec 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1185,29 +1179,38 @@ static int 
> libxl__build_device_model_args_new(libxl__gc *gc,
>                   * For other disks we translate devices 0..3 into
>                   * hd[a-d] and ignore the rest.
>                   */
> -                if (strncmp(disks[i].vdev, "sd", 2) == 0)
> +                if (strncmp(disks[i].vdev, "sd", 2) == 0) {
>                      drive = libxl__sprintf
> -                        (gc, 
> "file=%s,if=scsi,bus=0,unit=%d,format=%s,cache=writeback",
> -                         pdev_path, disk, format);
> -                else if (strncmp(disks[i].vdev, "xvd", 3) == 0)
> +                        (gc, 
> "file=%s,if=scsi,bus=0,unit=%d,format=%s,readonly=%s,cache=writeback",
> +                         pdev_path, disk, format, disks[i].readwrite ? "off" 
> : "on");
> +                } else if (strncmp(disks[i].vdev, "xvd", 3) == 0) {
>                      /*
>                       * Do not add any emulated disk when PV disk are
>                       * explicitly asked for.
>                       */
>                      continue;
> -                else if (disk < 6 && b_info->u.hvm.hdtype == 
> LIBXL_HDTYPE_AHCI) {
> +                } else if (disk < 6 && b_info->u.hvm.hdtype == 
> LIBXL_HDTYPE_AHCI) {
> +                    if (!disks[i].readwrite) {
> +                        LOG(ERROR, "qemu-xen doesn't support read-only AHCI 
> disk drivers");
> +                        return ERROR_INVAL;
> +                    }
>                      flexarray_vappend(dm_args, "-drive",
>                          
> GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
>                          pdev_path, disk, format),
>                          "-device", 
> GCSPRINTF("ide-hd,bus=ahci0.%d,unit=0,drive=ahcidisk-%d",
>                          disk, disk), NULL);
>                      continue;
> -                } else if (disk < 4)
> +                } else if (disk < 4) {
> +                    if (!disks[i].readwrite) {
> +                        LOG(ERROR, "qemu-xen doesn't support read-only IDE 
> disk drivers");
> +                        return ERROR_INVAL;
> +                    }
>                      drive = libxl__sprintf
>                          (gc, 
> "file=%s,if=ide,index=%d,media=disk,format=%s,cache=writeback",
>                           pdev_path, disk, format);
> -                else
> +                } else {
>                      continue; /* Do not emulate this disk */
> +                }
>              }
>  
>              flexarray_append(dm_args, "-drive");
> -- 
> 1.8.0.1
> 

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