diff --git a/docs/system/security.rst b/docs/system/security.rst index 52bbf0cc7a..af626a4230 100644 --- a/docs/system/security.rst +++ b/docs/system/security.rst @@ -133,6 +133,16 @@ an issue as a normal bug. that affect the level 0 QEMU process. While these bugs should be fixed, they will not be triaged as security flaws at this time. +* **migration/snapshots**. Migration failures and snapshot load + failures are considered part of normal operation as long as the + source virtual machine and savevm file, respectively, are still + functional. Aborting the QEMU process at the migration/snapshot + destination is similarly not considered a security issue. The + migration stream is assumed to be secure as long as the design + principles described in the Architecture section are held, in + which case plain manipulation of the stream is not considered as + an attack vector. + * **low severity impact**. As a catch all rule, issues which are judged to have a "low" severity impact on the system will usually not justify handling as security bugs, nor assignment @@ -159,10 +169,11 @@ could allow malicious guests to gain code execution in QEMU. At this point the guest has escaped the virtual machine and is able to act in the context of the QEMU process on the host. -Guests often interact with other guests and share resources with them. A -malicious guest must not gain control of other guests or access their data. -Disk image files and network traffic must be protected from other guests unless -explicitly shared between them by the user. +Guests often interact with other guests and share resources with them. +A malicious guest must not gain control of other guests or access +their data. Disk image files and network traffic must be protected +from other guests, users and processes unless explicitly shared with +them by the user. Principle of Least Privilege '''''''''''''''''''''''''''' @@ -223,6 +234,9 @@ Some Linux distros already ship with UNIX groups for these devices by default. system calls that are not needed by QEMU, thereby reducing the host kernel attack surface. +- Transport Layer Security (TLS) protocol can be used to ensure authenticity and + encryption of the live migration connection where the network is untrusted. + Sensitive configurations ------------------------ diff --git a/hmp-commands.hx b/hmp-commands.hx index 7ae2468a3d..7f43cf537f 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -928,16 +928,17 @@ ERST { .name = "migrate", - .args_type = "detach:-d,resume:-r,uri:s", - .params = "[-d] [-r] uri", + .args_type = "detach:-d,resume:-r,uri-cpr:-cs,uri:s", + .params = "[-d] [-r] [-c uri-cpr] uri", .help = "migrate to URI (using -d to not wait for completion)" - "\n\t\t\t -r to resume a paused postcopy migration", + "\n\t\t\t -r to resume a paused postcopy migration" + "\n\t\t\t -c to specify a CPR URI for cpr-transfer mode", .cmd = hmp_migrate, }, SRST -``migrate [-d] [-r]`` *uri* +``migrate [-d] [-r] [-c uri-cpr]`` *uri* Migrate the VM to *uri*. ``-d`` @@ -945,6 +946,9 @@ SRST query an ongoing migration process, use "info migrate". ``-r`` Resume a paused postcopy migration. + ``-c`` *uri-cpr* + Specify the CPR URI for cpr-transfer mode. It must be a UNIX domain + socket. ERST { diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c index 87fa7b6572..a0498d218f 100644 --- a/hw/remote/vfio-user-obj.c +++ b/hw/remote/vfio-user-obj.c @@ -375,9 +375,9 @@ static int vfu_object_mr_rw(MemoryRegion *mr, uint8_t *buf, hwaddr offset, ram_ptr = memory_region_get_ram_ptr(mr); if (is_write) { - memcpy((ram_ptr + offset), buf, size); + qemu_ram_move((ram_ptr + offset), buf, size); } else { - memcpy(buf, (ram_ptr + offset), size); + qemu_ram_move(buf, (ram_ptr + offset), size); } return 0; diff --git a/include/system/memory.h b/include/system/memory.h index 2192fc9bdc..1dc761058f 100644 --- a/include/system/memory.h +++ b/include/system/memory.h @@ -2668,6 +2668,39 @@ void address_space_register_map_client(AddressSpace *as, QEMUBH *bh); void address_space_unregister_map_client(AddressSpace *as, QEMUBH *bh); /* Internal functions, part of the implementation of address_space_read. */ + +/** + * qemu_ram_move: move data from or to ramblock + * + * @dst: destination where the data is moved to + * @src: source where the data is moved from + * @n: length of data to be moved + * + * Move @n bytes from @src to @dst, the memory areas may overlap. This + * provides the same semantics as memmove(), plus an additional stronger + * guarantee: if @n is 1, 2 or 4 or 8 bytes, and @src and @dst are both + * naturally aligned for that access size, then both the load and the store + * will be done as a single atomic access (with the semantics of + * qatomic_read() and qatomic_set()). + * + * This is the underlying function that we use to implement accesses by + * a guest vCPU or a device DMA operation to a ram block. The atomic + * guarantee is needed for two major cases: (A) When the ram block is + * backed by a PCI BAR passed through from a host device (and so it might + * be hardware registers that must be accessed exactly once at the right + * width); (B) When an emulated device updates a data structure shared in + * guest memory with guest software (e.g. a network device's set of tx and + * rx descriptor blocks), if a write to memory is accidentally performed + * multiple times then it can break the guest code when it busy polls the + * guest memory. + * + * We don't attempt to perform the exact access when it would be unaligned + * because this can't be done on all host architectures. Although this is + * strictly speaking not doing what would happen on real hardware, we don't + * think there are going to be situations where that matters in practice. + */ +void qemu_ram_move(void *dst, const void *src, size_t n); + MemTxResult address_space_read_full(const AddressSpace *as, hwaddr addr, MemTxAttrs attrs, void *buf, hwaddr len); MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, @@ -2685,15 +2718,8 @@ static inline bool memory_region_supports_direct_access(const MemoryRegion *mr) if (memory_region_is_romd(mr)) { return true; } - if (!memory_region_is_ram(mr)) { - return false; - } - /* - * RAM DEVICE regions can be accessed directly using memcpy, but it might - * be MMIO and access using mempy can be wrong (e.g., using instructions not - * intended for MMIO access). So we treat this as IO. - */ - return !memory_region_is_ram_device(mr); + + return memory_region_is_ram(mr); } static inline bool memory_access_is_direct(const MemoryRegion *mr, @@ -2741,7 +2767,7 @@ MemTxResult address_space_read(const AddressSpace *as, hwaddr addr, mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); if (len == l && memory_access_is_direct(mr, false, attrs)) { ptr = qemu_map_ram_ptr(mr->ram_block, addr1); - memcpy(buf, ptr, len); + qemu_ram_move(buf, ptr, len); } else { result = flatview_read_continue(fv, addr, attrs, buf, len, addr1, l, mr); diff --git a/migration/channel.c b/migration/channel.c index 1e2935f926..266ae8f776 100644 --- a/migration/channel.c +++ b/migration/channel.c @@ -296,9 +296,16 @@ int migration_channel_read_peek(QIOChannel *ioc, if (len == buflen) { break; + } else if (len == QIO_CHANNEL_ERR_BLOCK) { + qio_channel_wait_cond(ioc, G_IO_IN); + } else { + /* + * When partially ready, we can't use qio_channel_wait_cond() + * because it will return immediately. Apply a manual wait. + */ + assert(!qemu_in_coroutine()); + g_usleep(1000); } - - qio_channel_wait_cond(ioc, G_IO_IN); } return 0; diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index b04fc4489f..b5eb27467c 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -837,9 +837,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) bool detach = qdict_get_try_bool(qdict, "detach", false); bool resume = qdict_get_try_bool(qdict, "resume", false); const char *uri = qdict_get_str(qdict, "uri"); + const char *uri_cpr = qdict_get_try_str(qdict, "uri-cpr"); Error *err = NULL; g_autoptr(MigrationChannelList) caps = NULL; g_autoptr(MigrationChannel) channel = NULL; + g_autoptr(MigrationChannel) channel_cpr = NULL; if (!migrate_uri_parse(uri, &channel, &err)) { hmp_handle_error(mon, err); @@ -847,6 +849,22 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) } QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel)); + if (uri_cpr) { + if (migrate_mode() != MIG_MODE_CPR_TRANSFER) { + error_setg(&err, "-c can only be used in cpr-transfer mode"); + hmp_handle_error(mon, err); + return; + } + + if (!migrate_uri_parse(uri_cpr, &channel_cpr, &err)) { + hmp_handle_error(mon, err); + return; + } + + channel_cpr->channel_type = MIGRATION_CHANNEL_TYPE_CPR; + QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel_cpr)); + } + qmp_migrate(NULL, true, caps, true, resume, &err); if (hmp_handle_error(mon, err)) { return; diff --git a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c index 7419e5dc0d..0262e81eac 100644 --- a/migration/multifd-qatzip.c +++ b/migration/multifd-qatzip.c @@ -348,7 +348,10 @@ static int qatzip_recv(MultiFDRecvParams *p, Error **errp) multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } diff --git a/migration/multifd-qpl.c b/migration/multifd-qpl.c index 52902eb00c..3826e7f340 100644 --- a/migration/multifd-qpl.c +++ b/migration/multifd-qpl.c @@ -664,26 +664,42 @@ static int multifd_qpl_recv(MultiFDRecvParams *p, Error **errp) } multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } /* read compressed page lengths */ len = p->normal_num * sizeof(uint32_t); - assert(len < in_size); + if (len >= in_size) { + error_setg(errp, "multifd %u: header len %"PRIu32 + " >= packet size %"PRIu32, p->id, len, in_size); + return -1; + } ret = qio_channel_read_all(p->c, (void *) qpl->zlen, len, errp); if (ret != 0) { return ret; } for (int i = 0; i < p->normal_num; i++) { qpl->zlen[i] = be32_to_cpu(qpl->zlen[i]); - assert(qpl->zlen[i] <= multifd_ram_page_size()); + if (qpl->zlen[i] > multifd_ram_page_size()) { + error_setg(errp, "multifd %u: page %d compressed len %" + PRIu32" too large", p->id, i, qpl->zlen[i]); + return -1; + } zbuf_len += qpl->zlen[i]; ramblock_recv_bitmap_set_offset(p->block, p->normal[i]); } /* read compressed pages */ - assert(in_size == len + zbuf_len); + if (in_size != len + zbuf_len) { + error_setg(errp, "multifd %u: packet size %"PRIu32 + " != header %"PRIu32" + data %"PRIu32, + p->id, in_size, len, zbuf_len); + return -1; + } ret = qio_channel_read_all(p->c, (void *) qpl->zbuf, zbuf_len, errp); if (ret != 0) { return ret; diff --git a/migration/multifd-uadk.c b/migration/multifd-uadk.c index fd7cd9b5e8..d373615ba8 100644 --- a/migration/multifd-uadk.c +++ b/migration/multifd-uadk.c @@ -245,12 +245,19 @@ static int multifd_uadk_recv(MultiFDRecvParams *p, Error **errp) multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } /* read compressed data lengths */ - assert(hdr_len < in_size); + if (hdr_len >= in_size) { + error_setg(errp, "multifd %u: header len %"PRIu32 + " >= packet size %"PRIu32, p->id, hdr_len, in_size); + return -1; + } ret = qio_channel_read_all(p->c, (void *) uadk_data->buf_hdr, hdr_len, errp); if (ret != 0) { @@ -259,12 +266,21 @@ static int multifd_uadk_recv(MultiFDRecvParams *p, Error **errp) for (int i = 0; i < p->normal_num; i++) { uadk_data->buf_hdr[i] = be32_to_cpu(uadk_data->buf_hdr[i]); + if (uadk_data->buf_hdr[i] > page_size) { + error_setg(errp, "multifd %u: page %d compressed len %"PRIu32 + " too large", p->id, i, uadk_data->buf_hdr[i]); + return -1; + } data_len += uadk_data->buf_hdr[i]; - assert(uadk_data->buf_hdr[i] <= page_size); } /* read compressed data */ - assert(in_size == hdr_len + data_len); + if (in_size != hdr_len + data_len) { + error_setg(errp, "multifd %u: packet size %"PRIu32 + " != header %"PRIu32" + data %"PRIu32, + p->id, in_size, hdr_len, data_len); + return -1; + } ret = qio_channel_read_all(p->c, (void *)buf, data_len, errp); if (ret != 0) { return ret; diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c index 8820b2a787..400146566e 100644 --- a/migration/multifd-zlib.c +++ b/migration/multifd-zlib.c @@ -216,10 +216,19 @@ static int multifd_zlib_recv(MultiFDRecvParams *p, Error **errp) return -1; } + if (in_size > z->zbuff_len) { + error_setg(errp, "multifd %u: next_packet_size %"PRIu32 + " exceeds allocated %"PRIu32, p->id, in_size, z->zbuff_len); + return -1; + } + multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c index 3c2dcf76b0..69ef1a5f38 100644 --- a/migration/multifd-zstd.c +++ b/migration/multifd-zstd.c @@ -210,10 +210,19 @@ static int multifd_zstd_recv(MultiFDRecvParams *p, Error **errp) return -1; } + if (in_size > z->zbuff_len) { + error_setg(errp, "multifd %u: next_packet_size %"PRIu32 + " exceeds allocated %"PRIu32, p->id, in_size, z->zbuff_len); + return -1; + } + multifd_recv_zero_page_process(p); if (!p->normal_num) { - assert(in_size == 0); + if (in_size != 0) { + error_setg(errp, "multifd %u: expected empty packet", p->id); + return -1; + } return 0; } diff --git a/migration/ram.c b/migration/ram.c index 8918b2f03b..b6eb842746 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -4263,15 +4263,15 @@ static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length) return ret; } -static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes) +static int parse_ramblocks(QEMUFile *f, uint64_t total_ram_bytes) { int ret = 0; /* Synchronize RAM block list */ - while (!ret && total_ram_bytes) { + while (total_ram_bytes) { RAMBlock *block; char id[256]; - ram_addr_t length; + uint64_t length; int len = qemu_get_byte(f); qemu_get_buffer(f, (uint8_t *)id, len); @@ -4285,8 +4285,15 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes) error_report("Unknown ramblock \"%s\", cannot accept " "migration", id); ret = -EINVAL; + break; + } + + if (usub64_overflow(total_ram_bytes, length, &total_ram_bytes)) { + error_report("%s: RAMBlock '%s' size underflow total RAM size", + __func__, block->idstr); + ret = -EFAULT; + break; } - total_ram_bytes -= length; } return ret; diff --git a/system/memory.c b/system/memory.c index 5fc36708ec..da710bbade 100644 --- a/system/memory.c +++ b/system/memory.c @@ -1364,43 +1364,6 @@ const MemoryRegionOps unassigned_mem_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static uint64_t memory_region_ram_device_read(void *opaque, - hwaddr addr, unsigned size) -{ - MemoryRegion *mr = opaque; - uint64_t data = ldn_he_p(mr->ram_block->host + addr, size); - - trace_memory_region_ram_device_read(get_cpu_index(), mr, addr, data, size); - - return data; -} - -static void memory_region_ram_device_write(void *opaque, hwaddr addr, - uint64_t data, unsigned size) -{ - MemoryRegion *mr = opaque; - - trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size); - - stn_he_p(mr->ram_block->host + addr, size, data); -} - -static const MemoryRegionOps ram_device_mem_ops = { - .read = memory_region_ram_device_read, - .write = memory_region_ram_device_write, - .endianness = HOST_BIG_ENDIAN ? DEVICE_BIG_ENDIAN : DEVICE_LITTLE_ENDIAN, - .valid = { - .min_access_size = 1, - .max_access_size = 8, - .unaligned = true, - }, - .impl = { - .min_access_size = 1, - .max_access_size = 8, - .unaligned = true, - }, -}; - bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr, unsigned size, @@ -1692,10 +1655,8 @@ void memory_region_init_ram_device_ptr(MemoryRegion *mr, Object *owner, const char *name, uint64_t size, void *ptr) { - memory_region_init_io(mr, owner, &ram_device_mem_ops, mr, name, size); - mr->ram = true; + memory_region_init_ram_ptr(mr, owner, name, size, ptr); mr->ram_device = true; - memory_region_set_ram_ptr(mr, size, ptr); } void memory_region_init_alias(MemoryRegion *mr, Object *owner, diff --git a/system/physmem.c b/system/physmem.c index c21ea92915..2f37cbeb07 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -3158,6 +3158,50 @@ void memory_region_flush_rom_device(MemoryRegion *mr, hwaddr addr, hwaddr size) invalidate_and_set_dirty(mr, addr, size); } +void qemu_ram_move(void *dst, const void *src, size_t n) +{ + uintptr_t test, len; + + if (n == 0) { + return; + } + + /* + * Calculate "the lowest set bit" over @src, @dst and @n, result put + * into @len (which guarantees a power-of-two). With that and the + * later check (len!=n), it makes sure that we will only do the atomic + * ops when: + * + * (1) @n is a power-of-two + * (2) @src and @dst addresses are both aligned to @n + */ + test = (uintptr_t)src | (uintptr_t)dst | n; + len = test & -test; + + /* Overlapping buffers, unaligned or oversized access */ + if (n > 8 || len != n) { + memmove(dst, src, n); + return; + } + + switch (len) { + case 1: + qatomic_set((uint8_t *)dst, qatomic_read((uint8_t *)src)); + break; + case 2: + qatomic_set((uint16_t *)dst, qatomic_read((uint16_t *)src)); + break; + case 4: + qatomic_set((uint32_t *)dst, qatomic_read((uint32_t *)src)); + break; + case 8: + qatomic_set((uint64_t *)dst, qatomic_read((uint64_t *)src)); + break; + default: + g_assert_not_reached(); + } +} + int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) { unsigned access_size_max = mr->ops->valid.max_access_size; @@ -3270,7 +3314,7 @@ static MemTxResult flatview_write_continue_step(MemTxAttrs attrs, uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, false, true); - memmove(ram_ptr, buf, *l); + qemu_ram_move(ram_ptr, buf, *l); invalidate_and_set_dirty(mr, mr_addr, *l); return MEMTX_OK; @@ -3363,7 +3407,7 @@ static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf, uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, false, false); - memcpy(buf, ram_ptr, *l); + qemu_ram_move(buf, ram_ptr, *l); return MEMTX_OK; } diff --git a/system/trace-events b/system/trace-events index 51b4a4679a..d483b31419 100644 --- a/system/trace-events +++ b/system/trace-events @@ -20,8 +20,6 @@ memory_region_ops_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, u memory_region_ops_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size, const char *name) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u name '%s'" memory_region_subpage_read(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u" memory_region_subpage_write(int cpu_index, void *mr, uint64_t offset, uint64_t value, unsigned size) "cpu %d mr %p offset 0x%"PRIx64" value 0x%"PRIx64" size %u" -memory_region_ram_device_read(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" -memory_region_ram_device_write(int cpu_index, void *mr, uint64_t addr, uint64_t value, unsigned size) "cpu %d mr %p addr 0x%"PRIx64" value 0x%"PRIx64" size %u" memory_region_sync_dirty(const char *mr, const char *listener, int global) "mr '%s' listener '%s' synced (global=%d)" flatview_new(void *view, void *root) "%p (root %p)" flatview_destroy(void *view, void *root) "%p (root %p)" diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c index 827cc7bcf8..9bdb1165af 100644 --- a/tests/qtest/migration/tls-tests.c +++ b/tests/qtest/migration/tls-tests.c @@ -492,6 +492,7 @@ static void test_precopy_tcp_no_tls(char *name, MigrateCommon *args) test_precopy_common(args); } +#ifdef CONFIG_TASN1 static void * migrate_hook_start_tls_x509_no_host(QTestState *from, QTestState *to) { @@ -519,7 +520,6 @@ static void test_precopy_tcp_tls_no_hostname(char *name, MigrateCommon *args) test_precopy_common(args); } -#ifdef CONFIG_TASN1 static void test_precopy_tcp_tls_x509_default_host(char *name, MigrateCommon *args) { @@ -719,8 +719,10 @@ void migration_test_add_tls(MigrationTestEnv *env) migration_test_add("/migration/precopy/tcp/no-tls", test_precopy_tcp_no_tls); +#ifdef CONFIG_TASN1 migration_test_add("/migration/precopy/tcp/tls/no-hostname", test_precopy_tcp_tls_no_hostname); +#endif /* CONFIG_TASN1 */ migration_test_add("/migration/precopy/unix/tls/psk", test_precopy_unix_tls_psk);