[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v3 03/19] qemu-nbd: Sanity check partition bounds
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v3 03/19] qemu-nbd: Sanity check partition bounds |
Date: |
Sat, 12 Jan 2019 11:57:56 -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. Also, note that the
partition code blindly overwrites any offset passed in by the
user; so make the -o/-P combo an error for less confusion.
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>
---
v3: new patch
---
qemu-nbd.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 51b55f2e066..ff4adb9b3eb 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1013,12 +1013,28 @@ 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);
}
+ /* partition limits are (32-bit << 9); 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
- Re: [Qemu-devel] [PATCH v3 04/19] nbd/server: Hoist length check to qemp_nbd_server_add, (continued)
Re: [Qemu-devel] [PATCH v3 04/19] nbd/server: Hoist length check to qemp_nbd_server_add, Eric Blake, 2019/01/16
[Qemu-devel] [PATCH v3 05/19] nbd/server: Favor [u]int64_t over off_t, Eric Blake, 2019/01/12
[Qemu-devel] [PATCH v3 03/19] qemu-nbd: Sanity check partition bounds,
Eric Blake <=
[Qemu-devel] [PATCH v3 14/19] nbd/client: Pull out oldstyle size determination, Eric Blake, 2019/01/12
[Qemu-devel] [PATCH v3 08/19] nbd/client: Move export name into NBDExportInfo, Eric Blake, 2019/01/12