From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, berrange@redhat.com, linfeng23@huawei.com,
groug@kaod.org, huangy81@chinatelecom.cn, lvivier@redhat.com,
lizhijian@cn.fujitsu.com, peterx@redhat.com, vgoyal@redhat.com
Cc: leobras@redhat.com, stefanha@redhat.com, quintela@redhat.com
Subject: [PULL 02/19] migration: fix the memory overwriting risk in add_to_iovec
Date: Mon, 5 Jul 2021 11:02:18 +0100 [thread overview]
Message-ID: <20210705100235.157093-3-dgilbert@redhat.com> (raw)
In-Reply-To: <20210705100235.157093-1-dgilbert@redhat.com>
From: Feng Lin <linfeng23@huawei.com>
When testing migration, a Segmentation fault qemu core is generated.
0 error_free (err=0x1)
1 0x00007f8b862df647 in qemu_fclose (f=f@entry=0x55e06c247640)
2 0x00007f8b8516d59a in migrate_fd_cleanup (s=s@entry=0x55e06c0e1ef0)
3 0x00007f8b8516d66c in migrate_fd_cleanup_bh (opaque=0x55e06c0e1ef0)
4 0x00007f8b8626a47f in aio_bh_poll (ctx=ctx@entry=0x55e06b5a16d0)
5 0x00007f8b8626e71f in aio_dispatch (ctx=0x55e06b5a16d0)
6 0x00007f8b8626a33d in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>)
7 0x00007f8b866bdba4 in g_main_context_dispatch ()
8 0x00007f8b8626cde9 in glib_pollfds_poll ()
9 0x00007f8b8626ce62 in os_host_main_loop_wait (timeout=<optimized out>)
10 0x00007f8b8626cffd in main_loop_wait (nonblocking=nonblocking@entry=0)
11 0x00007f8b862ef01f in main_loop ()
Using gdb print the struct QEMUFile f = {
...,
iovcnt = 65, last_error = 21984,
last_error_obj = 0x1, shutdown = true
}
Well iovcnt is overflow, because the max size of MAX_IOV_SIZE is 64.
struct QEMUFile {
...;
struct iovec iov[MAX_IOV_SIZE];
unsigned int iovcnt;
int last_error;
Error *last_error_obj;
bool shutdown;
};
iovcnt and last_error is overwrited by add_to_iovec().
Right now, add_to_iovec() increase iovcnt before check the limit.
And it seems that add_to_iovec() assumes that iovcnt will set to zero
in qemu_fflush(). But qemu_fflush() will directly return when f->shutdown
is true.
The situation may occur when libvirtd restart during migration, after
f->shutdown is set, before calling qemu_file_set_error() in
qemu_file_shutdown().
So the safiest way is checking the iovcnt before increasing it.
Signed-off-by: Feng Lin <linfeng23@huawei.com>
Message-Id: <20210625062138.1899-1-linfeng23@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Fix typo in 'writeable' which is actually misnamed 'writable'
---
migration/qemu-file.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index d6e03dbc0e..1eacf9e831 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -416,6 +416,11 @@ static int add_to_iovec(QEMUFile *f, const uint8_t *buf, size_t size,
{
f->iov[f->iovcnt - 1].iov_len += size;
} else {
+ if (f->iovcnt >= MAX_IOV_SIZE) {
+ /* Should only happen if a previous fflush failed */
+ assert(f->shutdown || !qemu_file_is_writable(f));
+ return 1;
+ }
if (may_free) {
set_bit(f->iovcnt, f->may_free);
}
--
2.31.1
next prev parent reply other threads:[~2021-07-05 10:07 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-05 10:02 [PULL v2 00/19] migration queue Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 01/19] tests: migration-test: Add dirty ring test Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` Dr. David Alan Gilbert (git) [this message]
2021-07-05 10:02 ` [PULL 03/19] migration: Move yank outside qemu_start_incoming_migration() Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 04/19] migration: Allow reset of postcopy_recover_triggered when failed Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 05/19] migration: move wait-unplug loop to its own function Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 06/19] migration: failover: continue to wait card unplug on error Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 07/19] virtiofsd: use GDateTime for formatting timestamp for debug messages Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 08/19] docs: describe the security considerations with virtiofsd xattr mapping Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 09/19] virtiofsd: Don't allow file creation with FUSE_OPEN Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 10/19] virtiofsd: Fix fuse setxattr() API change issue Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 11/19] virtiofsd: Fix xattr operations overwriting errno Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 12/19] virtiofsd: Add support for extended setxattr Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 13/19] virtiofsd: Add umask to seccom allow list Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 14/19] virtiofsd: Add capability to change/restore umask Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 15/19] virtiofsd: Switch creds, drop FSETID for system.posix_acl_access xattr Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 16/19] virtiofsd: Add an option to enable/disable posix acls Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 17/19] tests/migration: parse the thread-id key of CpuInfoFast Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 18/19] tests/migration: fix "downtime_limit" type when "migrate-set-parameters" Dr. David Alan Gilbert (git)
2021-07-05 10:02 ` [PULL 19/19] migration/rdma: Use error_report to suppress errno message Dr. David Alan Gilbert (git)
2021-07-05 13:56 ` [PULL v2 00/19] migration queue Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210705100235.157093-3-dgilbert@redhat.com \
--to=dgilbert@redhat.com \
--cc=berrange@redhat.com \
--cc=groug@kaod.org \
--cc=huangy81@chinatelecom.cn \
--cc=leobras@redhat.com \
--cc=linfeng23@huawei.com \
--cc=lizhijian@cn.fujitsu.com \
--cc=lvivier@redhat.com \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=stefanha@redhat.com \
--cc=vgoyal@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).