From 58096dddd1682a83fb782a49d80199da9a117ef9 Mon Sep 17 00:00:00 2001

From: John Snow <jsnow@redhat.com>

Date: Wed, 27 Mar 2019 17:22:38 +0100

Subject: [PATCH 100/163] qemu-nbd: Sanity check partition bounds

RH-Author: John Snow <jsnow@redhat.com>

Message-id: <20190327172308.31077-26-jsnow@redhat.com>

Patchwork-id: 85207

O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 25/55] qemu-nbd: Sanity check partition bounds

Bugzilla: 1691009

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Max Reitz <mreitz@redhat.com>

RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>

From: Eric Blake <eblake@redhat.com>

When the user requests a partition, we were using data read

from the disk as disk offsets without a bounds check. We got

lucky that even when computed offsets are out-of-bounds,

blk_pread() will gracefully catch the error later (so I don't

think a malicious image can crash or exploit qemu-nbd, and am

not treating this as a security flaw), but it's better to

flag the problem up front than to risk permanent EIO death of

the block device down the road.  The new bounds check adds

an assertion that will never fail, but rather exists to help

the compiler see that adding two positive 41-bit values

(given MBR constraints) can't overflow 64-bit off_t.

Using off_t to represent a partition length is a bit of a

misnomer; a later patch will update to saner types, but it

is left separate in case the bounds check needs to be

backported in isolation.

Also, note that the partition code blindly overwrites any

non-zero offset passed in by the user; so for now, make the

-o/-P combo an error for less confusion.  In the future, we

may let -o and -P work together (selecting a subset of a

partition); so it is okay that an explicit '-o 0' behaves

no differently from omitting -o.

This can be tested with nbdkit:

$ echo hi > file

$ nbdkit -fv --filter=truncate partitioning file truncate=64k

Pre-patch:

$ qemu-nbd -p 10810 -P 1 -f raw nbd://localhost:10809 &

$ qemu-io -f raw nbd://localhost:10810

qemu-io> r -v 0 1

Disconnect client, due to: Failed to send reply: reading from file failed: Input/output error

Connection closed

read failed: Input/output error

qemu-io> q

[1]+  Done                    qemu-nbd -p 10810 -P 1 -f raw nbd://localhost:10809

Post-patch:

$ qemu-nbd -p 10810 -P 1 -f raw nbd://localhost:10809

qemu-nbd: Discovered partition 1 at offset 1048576 size 512, but size exceeds file length 65536

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

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Message-Id: <20190117193658.16413-5-eblake@redhat.com>

(cherry picked from commit 4485936b6de20afa38138e9d1e8ffed88cf0be73)

Signed-off-by: John Snow <jsnow@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 qemu-nbd.c | 22 +++++++++++++++++++++-

 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c

index 51b55f2..5c90c5e 100644

--- a/qemu-nbd.c

+++ b/qemu-nbd.c

@@ -1013,12 +1013,32 @@ int main(int argc, char **argv)

     fd_size -= dev_offset;

     if (partition != -1) {

-        ret = find_partition(blk, partition, &dev_offset, &fd_size);

+        off_t limit;

+

+        if (dev_offset) {

+            error_report("Cannot request partition and offset together");

+            exit(EXIT_FAILURE);

+        }

+        ret = find_partition(blk, partition, &dev_offset, &limit);

         if (ret < 0) {

             error_report("Could not find partition %d: %s", partition,

                          strerror(-ret));

             exit(EXIT_FAILURE);

         }

+        /*

+         * MBR partition limits are (32-bit << 9); this assert lets

+         * the compiler know that we have two positive values that

+         * can't overflow 64 bits.

+         */

+        assert(dev_offset >= 0 && dev_offset + limit >= dev_offset);

+        if (dev_offset + limit > fd_size) {

+            error_report("Discovered partition %d at offset %lld size %lld, "

+                         "but size exceeds file length %lld", partition,

+                         (long long int) dev_offset, (long long int) limit,

+                         (long long int) fd_size);

+            exit(EXIT_FAILURE);

+        }

+        fd_size = limit;

     }

     export = nbd_export_new(bs, dev_offset, fd_size, export_name,

-- 

1.8.3.1