[Qemu-devel] [PATCH] qcow2: Prevent backing file names longer than 1023

[Qemu-devel] [PATCH] qcow2: Prevent backing file names longer than 1023

[Max Reitz]

We reject backing file names with a length of more than 1023 characters
when opening a qcow2 file, so we should not produce such files
ourselves.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 056525c..011a0ae 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1957,6 +1957,11 @@ int qcow2_update_header(BlockDriverState *bs)
     if (s->image_backing_file) {
         size_t backing_file_len = strlen(s->image_backing_file);
 
+        if (backing_file_len > 1023) {
+            ret = -EINVAL;
+            goto fail;
+        }
+
         if (buflen < backing_file_len) {
             ret = -ENOSPC;
             goto fail;
-- 
2.8.0

Re: [Qemu-devel] [PATCH] qcow2: Prevent backing file names longer than 1023

[Eric Blake]

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

On 04/06/2016 09:14 AM, Max Reitz wrote:
> We reject backing file names with a length of more than 1023 characters
> when opening a qcow2 file, so we should not produce such files
> ourselves.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] qcow2: Prevent backing file names longer than 1023

[Kevin Wolf]

Am 06.04.2016 um 17:14 hat Max Reitz geschrieben:
> We reject backing file names with a length of more than 1023 characters
> when opening a qcow2 file, so we should not produce such files
> ourselves.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 056525c..011a0ae 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1957,6 +1957,11 @@ int qcow2_update_header(BlockDriverState *bs)
>      if (s->image_backing_file) {
>          size_t backing_file_len = strlen(s->image_backing_file);
>  
> +        if (backing_file_len > 1023) {
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +
>          if (buflen < backing_file_len) {
>              ret = -ENOSPC;
>              goto fail;

We should probably already check this in qcow2_change_backing_file(), so
that s->image_backing_file can never contain anything longer than that.
If you like, you can keep an assertion here.

The advantage is that in qcow2_change_backing_file() we can fail the
operation before all of the variables are updated and therefore become
inconsistent with the on-disk state of the image.

Kevin