diff mbox

qcow2: Prevent backing file names longer than 1023

Message ID 1459955675-24011-1-git-send-email-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Max Reitz April 6, 2016, 3:14 p.m. UTC
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(+)

Comments

Eric Blake April 6, 2016, 3:36 p.m. UTC | #1
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>
Kevin Wolf April 6, 2016, 4:18 p.m. UTC | #2
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
diff mbox

Patch

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;