linux-ext4 - Re: [PATCH 2/3 v2] e2image: Support for conversion QCOW2 image into raw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 17 May 2011 13:10:54 +0200 (CEST)
From:	Lukas Czerner <lczerner@...hat.com>
To:	Andreas Dilger <adilger@...ger.ca>
cc:	Lukas Czerner <lczerner@...hat.com>, linux-ext4@...r.kernel.org,
	tytso@....edu, sandeen@...hat.com
Subject: Re: [PATCH 2/3 v2] e2image: Support for conversion QCOW2 image into
 raw

On Mon, 16 May 2011, Andreas Dilger wrote:

> On May 16, 2011, at 09:43, Lukas Czerner wrote:
> > diff --git a/lib/ext2fs/qcow2.c b/lib/ext2fs/qcow2.c
> > index 17eab38..9ac050b 100644
> > --- a/lib/ext2fs/qcow2.c
> > +++ b/lib/ext2fs/qcow2.c
> > @@ -209,9 +209,10 @@ int qcow2_write_raw_image(int qcow2_fd, int raw_fd,
> > 	}
> > 
> > 	/* Resize the output image to the filesystem size */
> > -	if (lseek(raw_fd, img.image_size, SEEK_SET) < 0)
> > +	if (lseek(raw_fd, img.image_size - 1, SEEK_SET) < 0)
> > 		return errno;
> > 
> > +	memset(copy_buf, 0, 1);
> > 	size = write(raw_fd, copy_buf, 1);
> > 	if (size != 1)
> > 		return errno;
> 
> This should probably be part of the first patch.  Probably memset() is overkill here, just "copy_buf[0] = 0;" is enough.

Right, thanks.
> 
> > diff --git a/misc/e2image.c b/misc/e2image.c
> > index 6dc78d3..d749981 100644
> > --- a/misc/e2image.c
> > +++ b/misc/e2image.c
> > @@ -1229,16 +1229,33 @@ static void install_image(char *device, char *image_fn, int type)
> > 	exit (0);
> > }
> > 
> > +static struct ext2_qcow2_hdr *check_qcow2_image(int *fd, char *name)
> > +{
> > +
> > +#ifdef HAVE_OPEN64
> > +	*fd = open64(name, O_RDONLY, 0600);
> > +#else
> > +	*fd = open(name, O_RDONLY, 0600);
> > +#endif
> > +	if (*fd < 0)
> > +		return NULL;
> > +
> > +	return qcow2_read_header(*fd, name);
> 
> It probably makes sense to check the header here that it is not using AES encryption, and return an error (NULL?) if it is.  I don't think there is a flag in the header if compression is used, since that is only marked in each block.
> 
> > +	if (flags & IS_QCOW2_FLAG) {
> > +		ret = qcow2_write_raw_image(qcow2_fd, fd, header);
> 
> This also reminds me that qcow2_write_raw_image() should check the block numbers read from disk whether they point to zlib-compressed blocks, and return an error if they do, instead of just writing out the compressed data to disk.  Otherwise, some users might accidentally try to "restore" the qcow2 image created by some other tool and corrupt their filesystem.

I'll check for the encryption and compression in qcow2_write_raw_image()
so it can return an corresponding error code (QCOW_COMPRESSED,
QCOW_ENCRYPTED).

> 
> Cheers, Andreas

Once again thanks for the review!

-Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html