From: Peter Krempa <pkrempa@redhat.com>

External inactive snapshots are created by invoking 'qemu-img' which
creates the file. Currently qemu-img creates image with mode 644 based
on default umask as libvirt doesn't set any.

Having a world-readable image is obviously wrong so set the umask to
0066 to have the file readable only by the owner.

Resolves: https://bugs.debian.org/1120119
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_snapshot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index d4994dd54e..6868910d9a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -228,6 +228,9 @@ qemuSnapshotCreateQcow2Files(virDomainDef *def,
                                          NULL)))
             return -1;

+        /* ensure that new files are only readable by the user */
+        virCommandSetUmask(cmd, 0066);
+
         /* adds cmd line arg: backing_fmt=format,backing_file=/path/to/backing/file */
         virBufferAsprintf(&buf, "backing_fmt=%s,backing_file=",
                           virStorageFileFormatTypeToString(defdisk->src->format));
-- 
2.51.1

On Wed, Nov 12, 2025 at 05:54:18PM +0100, Peter Krempa via Devel wrote:
> External inactive snapshots are created by invoking 'qemu-img' which
> creates the file. Currently qemu-img creates image with mode 644 based
> on default umask as libvirt doesn't set any.
>
> Having a world-readable image is obviously wrong so set the umask to
> 0066 to have the file readable only by the owner.
>
> Resolves: https://bugs.debian.org/1120119
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_snapshot.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index d4994dd54e..6868910d9a 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -228,6 +228,9 @@ qemuSnapshotCreateQcow2Files(virDomainDef *def,
>                                           NULL)))
>              return -1;
>
> +        /* ensure that new files are only readable by the user */
> +        virCommandSetUmask(cmd, 0066);

Does what it says on the tin.

I would argue we could go for 0077 instead of 0066, just to be super
duper safe, but I imagine that qemu-img will never set the executable
bit so effectively there's little need for it.

Whether or not the umask is changed to 0077

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Nov 12, 2025 at 09:22:20 -0800, Andrea Bolognani wrote:
> On Wed, Nov 12, 2025 at 05:54:18PM +0100, Peter Krempa via Devel wrote:
> > External inactive snapshots are created by invoking 'qemu-img' which
> > creates the file. Currently qemu-img creates image with mode 644 based
> > on default umask as libvirt doesn't set any.
> >
> > Having a world-readable image is obviously wrong so set the umask to
> > 0066 to have the file readable only by the owner.
> >
> > Resolves: https://bugs.debian.org/1120119
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/qemu/qemu_snapshot.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> > index d4994dd54e..6868910d9a 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> > @@ -228,6 +228,9 @@ qemuSnapshotCreateQcow2Files(virDomainDef *def,
> >                                           NULL)))
> >              return -1;
> >
> > +        /* ensure that new files are only readable by the user */
> > +        virCommandSetUmask(cmd, 0066);
> 
> Does what it says on the tin.
> 
> I would argue we could go for 0077 instead of 0066, just to be super
> duper safe, but I imagine that qemu-img will never set the executable
> bit so effectively there's little need for it.
> 
> Whether or not the umask is changed to 0077

I went with 077 as it seems to be the more popular option.

This code path isn't supposed to create directories anyways so there's
no actual difference.