[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 35/73] raw: Prohibit dangerous writes for probed imag
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PULL 35/73] raw: Prohibit dangerous writes for probed images |
Date: |
Wed, 10 Dec 2014 11:34:01 +0100 |
If the user neglects to specify the image format, QEMU probes the
image to guess it automatically, for convenience.
Relying on format probing is insecure for raw images (CVE-2008-2004).
If the guest writes a suitable header to the device, the next probe
will recognize a format chosen by the guest. A malicious guest can
abuse this to gain access to host files, e.g. by crafting a QCOW2
header with backing file /etc/shadow.
Commit 1e72d3b (April 2008) provided -drive parameter format to let
users disable probing. Commit f965509 (March 2009) extended QCOW2 to
optionally store the backing file format, to let users disable backing
file probing. QED has had a flag to suppress probing since the
beginning (2010), set whenever a raw backing file is assigned.
All of these additions that allow to avoid format probing have to be
specified explicitly. The default still allows the attack.
In order to fix this, commit 79368c8 (July 2010) put probed raw images
in a restricted mode, in which they wouldn't be able to overwrite the
first few bytes of the image so that they would identify as a different
image. If a write to the first sector would write one of the signatures
of another driver, qemu would instead zero out the first four bytes.
This patch was later reverted in commit 8b33d9e (September 2010) because
it didn't get the handling of unaligned qiov members right.
Today's block layer that is based on coroutines and has qiov utility
functions makes it much easier to get this functionality right, so this
patch implements it.
The other differences of this patch to the old one are that it doesn't
silently write something different than the guest requested by zeroing
out some bytes (it fails the request instead) and that it doesn't
maintain a list of signatures in the raw driver (it calls the usual
probe function instead).
Note that this change doesn't introduce new breakage for false positive
cases where the guest legitimately writes data into the first sector
that matches the signatures of an image format (e.g. for nested virt):
These cases were broken before, only the failure mode changes from
corruption after the next restart (when the wrong format is probed) to
failing the problematic write request.
Also note that like in the original patch, the restrictions only apply
if the image format has been guessed by probing. Explicitly specifying a
format allows guests to write anything they like.
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Message-id: address@hidden
Signed-off-by: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
block.c | 5 ++--
block/raw_bsd.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++-
include/block/block_int.h | 3 +++
3 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 809ec54..35f7a0a 100644
--- a/block.c
+++ b/block.c
@@ -662,8 +662,8 @@ BlockDriver *bdrv_find_protocol(const char *filename,
* probing score.
* Return the first block driver with the highest probing score.
*/
-static BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
- const char *filename)
+BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
+ const char *filename)
{
int score_max = 0, score;
BlockDriver *drv = NULL, *d;
@@ -1489,6 +1489,7 @@ int bdrv_open(BlockDriverState **pbs, const char
*filename,
}
/* Image format probing */
+ bs->probed = !drv;
if (!drv && file) {
ret = find_image_format(file, filename, &drv, &local_err);
if (ret < 0) {
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..2ce5409 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -58,8 +58,58 @@ static int coroutine_fn raw_co_readv(BlockDriverState *bs,
int64_t sector_num,
static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
+ void *buf = NULL;
+ BlockDriver *drv;
+ QEMUIOVector local_qiov;
+ int ret;
+
+ if (bs->probed && sector_num == 0) {
+ /* As long as these conditions are true, we can't get partial writes to
+ * the probe buffer and can just directly check the request. */
+ QEMU_BUILD_BUG_ON(BLOCK_PROBE_BUF_SIZE != 512);
+ QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512);
+
+ if (nb_sectors == 0) {
+ /* qemu_iovec_to_buf() would fail, but we want to return success
+ * instead of -EINVAL in this case. */
+ return 0;
+ }
+
+ buf = qemu_try_blockalign(bs->file, 512);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ ret = qemu_iovec_to_buf(qiov, 0, buf, 512);
+ if (ret != 512) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ drv = bdrv_probe_all(buf, 512, NULL);
+ if (drv != bs->drv) {
+ ret = -EPERM;
+ goto fail;
+ }
+
+ /* Use the checked buffer, a malicious guest might be overwriting its
+ * original buffer in the background. */
+ qemu_iovec_init(&local_qiov, qiov->niov + 1);
+ qemu_iovec_add(&local_qiov, buf, 512);
+ qemu_iovec_concat(&local_qiov, qiov, 512, qiov->size - 512);
+ qiov = &local_qiov;
+ }
+
BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
- return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
+ ret = bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
+
+fail:
+ if (qiov == &local_qiov) {
+ qemu_iovec_destroy(&local_qiov);
+ }
+ qemu_vfree(buf);
+ return ret;
}
static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
@@ -158,6 +208,18 @@ static int raw_open(BlockDriverState *bs, QDict *options,
int flags,
Error **errp)
{
bs->sg = bs->file->sg;
+
+ if (bs->probed && !bdrv_is_read_only(bs)) {
+ fprintf(stderr,
+ "WARNING: Image format was not specified for '%s' and probing "
+ "guessed raw.\n"
+ " Automatically detecting the format is dangerous for "
+ "raw images, write operations on block 0 will be restricted.\n"
+ " Specify the 'raw' format explicitly to remove the "
+ "restrictions.\n",
+ bs->file->filename);
+ }
+
return 0;
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cd94559..192fce8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -326,6 +326,7 @@ struct BlockDriverState {
int sg; /* if true, the device is a /dev/sg* */
int copy_on_read; /* if true, copy read backing sectors into image
note this is a reference count */
+ bool probed;
BlockDriver *drv; /* NULL means no media */
void *opaque;
@@ -414,6 +415,8 @@ struct BlockDriverState {
};
int get_tmp_filename(char *filename, int size);
+BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
+ const char *filename);
void bdrv_set_io_limits(BlockDriverState *bs,
ThrottleConfig *cfg);
--
1.8.3.1
- [Qemu-devel] [PULL 26/73] nbd: Use BlockBackend internally, (continued)
- [Qemu-devel] [PULL 26/73] nbd: Use BlockBackend internally, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 27/73] qemu-nbd: Use BlockBackend where reasonable, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 28/73] tests: Use "command -v" instead of which(1) in shell scripts, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 30/73] qemu-iotests: Use qemu-io -f $IMGFMT, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 31/73] qemu-iotests: Add qemu-io format option in Python tests, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 29/73] qemu-io: Allow explicitly specifying format, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 32/73] qtests: Specify image format explicitly, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 34/73] block: Read only one sector for format probing, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 37/73] qemu-iotests: Test writing non-raw image headers to raw image, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 36/73] qemu-iotests: Fix stderr handling in common.qemu, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 35/73] raw: Prohibit dangerous writes for probed images,
Kevin Wolf <=
- [Qemu-devel] [PULL 40/73] blockdev: acquire AioContext in QMP 'transaction' actions, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 42/73] qcow2: Fix header extension size check, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 39/73] blockdev: drop unnecessary DriveBackupState field assignment, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 41/73] blockdev: check for BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 38/73] blockdev: update outdated qmp_transaction() comments, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 44/73] block: Don't probe for unknown backing file format, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 46/73] qemu-iotests: 060: Filter the real disk size, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 45/73] block: do not use get_clock(), Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 48/73] nvme: 64kB page size fixes, Kevin Wolf, 2014/12/10
- [Qemu-devel] [PULL 47/73] qemu-iotests: 082: Filter the real disk size, Kevin Wolf, 2014/12/10