[libvirt] [PATCH] util: storage: drop VIR_STORAGE_FILE_AUTO_SAFE

Yi Li posted 1 patch 4 years, 8 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1566791946-21684-1-git-send-email-yili@winhong.com
src/qemu/qemu_block.c     | 2 --
src/util/virstoragefile.c | 4 +---
src/util/virstoragefile.h | 1 -
3 files changed, 1 insertion(+), 6 deletions(-)
[libvirt] [PATCH] util: storage: drop VIR_STORAGE_FILE_AUTO_SAFE
Posted by Yi Li 4 years, 8 months ago
merge VIR_STORAGE_FILE_AUTO_SAFE/VIR_STORAGE_FILE_AUTO to VIR_STORAGE_FILE_AUTO
virStorageFileProbeFormatFromBuf will probe the backingStore format.

Fix the booting issue when setting backingStore format (QCOW image) to RAW image.

Signed-off-by: Yi Li <yili@winhong.com>
---
 src/qemu/qemu_block.c     | 2 --
 src/util/virstoragefile.c | 4 +---
 src/util/virstoragefile.h | 1 -
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 66b1d11..0b99efc 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1367,7 +1367,6 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src)
         driver = virStorageFileFormatTypeToString(src->format);
         break;
 
-    case VIR_STORAGE_FILE_AUTO_SAFE:
     case VIR_STORAGE_FILE_AUTO:
     case VIR_STORAGE_FILE_NONE:
     case VIR_STORAGE_FILE_COW:
@@ -2275,7 +2274,6 @@ qemuBlockStorageSourceCreateGetFormatProps(virStorageSourcePtr src,
     case VIR_STORAGE_FILE_DIR:
         return 0;
 
-    case VIR_STORAGE_FILE_AUTO_SAFE:
     case VIR_STORAGE_FILE_AUTO:
     case VIR_STORAGE_FILE_NONE:
         virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 7288e18..2ebb2f1 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -692,7 +692,7 @@ qedGetBackingStore(char **res,
     if (flags & QED_F_BACKING_FORMAT_NO_PROBE)
         *format = VIR_STORAGE_FILE_RAW;
     else
-        *format = VIR_STORAGE_FILE_AUTO_SAFE;
+        *format = VIR_STORAGE_FILE_AUTO;
 
     return BACKING_STORE_OK;
 }
@@ -4916,8 +4916,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
             goto cleanup;
 
         if (backingFormat == VIR_STORAGE_FILE_AUTO)
-            backingStore->format = VIR_STORAGE_FILE_RAW;
-        else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
             backingStore->format = VIR_STORAGE_FILE_AUTO;
         else
             backingStore->format = backingFormat;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index b65cd4c..c9deb6f 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -60,7 +60,6 @@ VIR_ENUM_DECL(virStorage);
 
 
 typedef enum {
-    VIR_STORAGE_FILE_AUTO_SAFE = -2,
     VIR_STORAGE_FILE_AUTO = -1,
     VIR_STORAGE_FILE_NONE = 0,
     VIR_STORAGE_FILE_RAW,
-- 
2.7.5



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: storage: drop VIR_STORAGE_FILE_AUTO_SAFE
Posted by Peter Krempa 4 years, 8 months ago
On Mon, Aug 26, 2019 at 11:59:06 +0800, Yi Li wrote:
> merge VIR_STORAGE_FILE_AUTO_SAFE/VIR_STORAGE_FILE_AUTO to VIR_STORAGE_FILE_AUTO
> virStorageFileProbeFormatFromBuf will probe the backingStore format.
> 
> Fix the booting issue when setting backingStore format (QCOW image) to RAW image.

This description does not really describe what the problem is.

> 
> Signed-off-by: Yi Li <yili@winhong.com>
> ---
>  src/qemu/qemu_block.c     | 2 --
>  src/util/virstoragefile.c | 4 +---
>  src/util/virstoragefile.h | 1 -
>  3 files changed, 1 insertion(+), 6 deletions(-)
> 
> @@ -4916,8 +4916,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
>              goto cleanup;
>  
>          if (backingFormat == VIR_STORAGE_FILE_AUTO)
> -            backingStore->format = VIR_STORAGE_FILE_RAW;
> -        else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
>              backingStore->format = VIR_STORAGE_FILE_AUTO;

I don't think we can do this safely. This code was added so that we
never let qemu probe the image format. This was due to a security issue
as a malicious guest could write a qcow2 or any other storage format
header which has backing files into a raw volume. At new start this
would be detected as the qcow2 or other format and qemu would open also
the backing file. The guest then would gain access to un-allowed
resources.

While now qemu refuses writing some parts of the raw image if no format
was specified I don't think we should remove this code. Users always
shall use the correct format.

NACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list