[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 10/24] migration: In case of error just end
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH v10 10/24] migration: In case of error just end the migration |
Date: |
Wed, 7 Mar 2018 11:52:10 +0000 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Wed, Mar 07, 2018 at 11:59:56AM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <address@hidden>
> ---
> migration/socket.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <address@hidden>
> diff --git a/migration/socket.c b/migration/socket.c
> index 08606c665d..b12b0a462e 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -139,9 +139,8 @@ static gboolean
> socket_accept_incoming_migration(QIOChannel *ioc,
> sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
> &err);
> if (!sioc) {
> - error_report("could not accept migration connection (%s)",
> - error_get_pretty(err));
> - goto out;
> + migrate_set_error(migrate_get_current(), err);
> + return G_SOURCE_REMOVE;
It will only return NULL if a client connected & then went away. This should
not happen with a "normal" mgmt app usage. On the flip side this allows a
malicious network attacker to inflict a denial of service on the migration
by simply connecting to target QEMU & immediately exiting.
Our "authentication" for migration relies on being able to validate the TLS
certs during TLS handshake. So in general we ought to allow repeated incoming
connections until we get a successful handshake.
So in fact, I think a better fix here is to simply remove the original
'error_report' line, and ensure we return G_SOURCE_CONTINUE to wait for
another incoming connection from the real mgmt app.
> }
>
> trace_migration_socket_incoming_accepted();
> @@ -150,7 +149,6 @@ static gboolean
> socket_accept_incoming_migration(QIOChannel *ioc,
> migration_channel_process_incoming(QIO_CHANNEL(sioc));
> object_unref(OBJECT(sioc));
>
> -out:
> if (migration_has_all_channels()) {
> /* Close listening socket as its no longer needed */
> qio_channel_close(ioc, NULL);
> --
> 2.14.3
>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [Qemu-devel] [PATCH v10 12/24] migration: Reference counting recv channels correctly, (continued)
- [Qemu-devel] [PATCH v10 03/24] migration: Create tcp_port parameter, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 07/24] [RFH] tests: Add migration compress threads tests, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 06/24] tests: Add basic migration precopy tcp test, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 11/24] migration: terminate_* can be called for other threads, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 14/24] migration: Be sure all recv channels are created, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 01/24] tests: Add migration precopy test, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 10/24] migration: In case of error just end the migration, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 02/24] tests: Add migration xbzrle test, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 13/24] migration: Introduce multifd_recv_new_channel(), Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 18/24] migration: Add multifd traces for start/end thread, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 16/24] migration: Synchronize recv threads, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 15/24] migration: Synchronize send threads, Juan Quintela, 2018/03/07
- [Qemu-devel] [PATCH v10 19/24] migration: Create multifd channels, Juan Quintela, 2018/03/07