[RFC] libxl: relax readonly check introduced by XSA-142 fix

[Jim Fehlig <jfehlig@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

Hi All,

Apologies for only noticing the fix for XSA-142 as it starting flowing to our
various downstreams. The current fix seems like quite a big hammer IMO. qemu
doesn't support readonly IDE/SATA 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

But 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 skipped.

The attached follow-up loosens the restriction to reject readonly when creating
and emulated IDE disk, but allows it when the backend is known to support readonly.

Regards,
Jim

[-- Attachment #2: relax-xsa142-readonly-check.patch --]
[-- Type: text/x-diff, Size: 4731 bytes --]

>From 3d58f93b58ab544b9ee168b7bbaf59de1c5532ce Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Tue, 10 Nov 2015 11:33:44 -0700
Subject: [PATCH] libxl: relax readonly check introduced by XSA-142 fix

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 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

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 disk, but allows it when the backend is
known to support readonly.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 tools/libxl/libxl_dm.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 9c9eaa3..ccfc827 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1152,12 +1152,6 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
                         (gc, "file=%s,if=ide,index=%d,readonly=%s,media=cdrom,format=%s,cache=writeback,id=ide-%i",
                          disks[i].pdev_path, disk, disks[i].readwrite ? "off" : "on", format, dev_number);
             } else {
-                if (!disks[i].readwrite) {
-                    LOG(ERROR,
-                        "qemu-xen doesn't support read-only disk drivers");
-                    return ERROR_INVAL;
-                }
-
                 if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
                     LOG(WARN, "cannot support"" empty disk format for %s",
                         disks[i].vdev);
@@ -1185,29 +1179,34 @@ 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) {
                     flexarray_vappend(dm_args, "-drive",
-                        GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,cache=writeback",
-                        pdev_path, disk, format),
+                        GCSPRINTF("file=%s,if=none,id=ahcidisk-%d,format=%s,readonly=%s,cache=writeback",
+                                  pdev_path, disk, format, disks[i].readwrite ? "off" : "on"),
                         "-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


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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