[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/5] migration: Fix race on qemu_file_shutdown()
From: |
Juan Quintela |
Subject: |
Re: [PATCH v2 2/5] migration: Fix race on qemu_file_shutdown() |
Date: |
Mon, 14 Nov 2022 15:02:50 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) |
Peter Xu <peterx@redhat.com> wrote:
> In qemu_file_shutdown(), there's a possible race if with current order of
> operation. There're two major things to do:
>
> (1) Do real shutdown() (e.g. shutdown() syscall on socket)
> (2) Update qemufile's last_error
>
> We must do (2) before (1) otherwise there can be a race condition like:
>
> page receiver other thread
> ------------- ------------
> qemu_get_buffer()
> do shutdown()
> returns 0 (buffer all zero)
> (meanwhile we didn't check this retcode)
> try to detect IO error
> last_error==NULL, IO okay
> install ALL-ZERO page
> set last_error
> --> guest crash!
>
> To fix this, we can also check retval of qemu_get_buffer(), but not all
> APIs can be properly checked and ultimately we still need to go back to
> qemu_file_get_error(). E.g. qemu_get_byte() doesn't return error.
>
> Maybe some day a rework of qemufile API is really needed, but for now keep
> using qemu_file_get_error() and fix it by not allowing that race condition
> to happen. Here shutdown() is indeed special because the last_error was
> emulated. For real -EIO errors it'll always be set when e.g. sendmsg()
> error triggers so we won't miss those ones, only shutdown() is a bit tricky
> here.
>
> Cc: Daniel P. Berrange <berrange@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 2/5] migration: Fix race on qemu_file_shutdown(),
Juan Quintela <=