[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 04/21] qemu-nbd: Sanity check partition bounds
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PULL 04/21] qemu-nbd: Sanity check partition bounds |
Date: |
Mon, 21 Jan 2019 16:48:50 -0600 |
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 <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Richard W.M. Jones <address@hidden>
Message-Id: <address@hidden>
---
qemu-nbd.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 51b55f2e066..5c90c5e55f7 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,
--
2.20.1
- [Qemu-devel] [PULL 00/21] NBD patches through 2019-01-21, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 02/21] maint: Allow for EXAMPLES in texi2pod, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 01/21] iotests: Make 233 output more reliable, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 03/21] qemu-nbd: Enhance man page, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 04/21] qemu-nbd: Sanity check partition bounds,
Eric Blake <=
- [Qemu-devel] [PULL 05/21] nbd/server: Hoist length check to qmp_nbd_server_add, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 17/21] nbd/client: Add nbd_receive_export_list(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 08/21] nbd/client: Refactor nbd_receive_list(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 16/21] nbd/client: Refactor nbd_opt_go() to support NBD_OPT_INFO, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 06/21] nbd/server: Favor [u]int64_t over off_t, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 14/21] nbd/client: Split handshake into two functions, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 09/21] nbd/client: Move export name into NBDExportInfo, Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 11/21] nbd/client: Split out nbd_send_meta_query(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 18/21] nbd/client: Add meta contexts to nbd_receive_export_list(), Eric Blake, 2019/01/21
- [Qemu-devel] [PULL 12/21] nbd/client: Split out nbd_receive_one_meta_context(), Eric Blake, 2019/01/21