[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [PATCH 3/8] xen: defer call to xen_restrict
From: |
Andrew Cooper |
Subject: |
Re: [Qemu-devel] [Xen-devel] [PATCH 3/8] xen: defer call to xen_restrict until just before os_setup_post |
Date: |
Fri, 13 Oct 2017 09:59:59 +0100 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 13/10/2017 09:37, Ross Lagerwall wrote:
> On 10/09/2017 05:01 PM, Ian Jackson wrote:
>> We need to restrict *all* the control fds that qemu opens. Looking in
>> /proc/PID/fd shows there are many; their allocation seems scattered
>> throughout Xen support code in qemu.
>>
>> We must postpone the restrict call until roughly the same time as qemu
>> changes its uid, chroots (if applicable), and so on.
>>
>> There doesn't seem to be an appropriate hook already. The RunState
>> change hook fires at different times depending on exactly what mode
>> qemu is operating in.
>>
>> And it appears that no-one but the Xen code wants a hook at this phase
>> of execution. So, introduce a bare call to a new function
>> xen_setup_post, just before os_setup_post. Also provide the
>> appropriate stub for when Xen compilation is disabled.
>>
>> We do the restriction before rather than after os_setup_post, because
>> xen_restrict may need to open /dev/null, and os_setup_post might have
>> called chroot.
>>
> This works for normally starting a VM but doesn't seem to work when
> resuming/migrating.
>
> Here is the order of selected operations when starting a VM normally:
> VNC server running on 127.0.0.1:5901
> xen_change_state_handler()
> xenstore_record_dm_state: running
> xen_setup_post()
> xentoolcore_restrict_all: rc = 0
> os_setup_post()
> main_loop()
>
> Here is the order of selected operations when starting QEMU with
> -incoming fd:... :
> VNC server running on 127.0.0.1:5902
> migration_fd_incoming()
> xen_setup_post()
> xentoolcore_restrict_all: rc = 0
> os_setup_post()
> main_loop()
> migration_set_incoming_channel()
> migrate_set_state()
> xen_change_state_handler()
> xenstore_record_dm_state: running
> error recording dm state
> qemu exited with code 1
>
> The issue is that QEMU needs xenstore access to write "running" but
> this is after it has already been restricted. Moving xen_setup_post()
> into xen_change_state_handler() works fine. The only issue is that in
> the migration case, it executes after os_setup_post() so QEMU might be
> chrooted in which case opening /dev/null to restrict fds doesn't work
> (unless its new root has a /dev/null).
>
Wasn't the agreement in the end to remove all use of xenstore from the
device mode? This running notification can and should be QMP, at which
point we break a causal dependency.
For safety reasons, qemu needs to have restricted/dropped/etc all
permissions before it looks at a single byte of incoming migration data,
to protect against buggy or malicious alterations to the migration stream.
~Andrew
- [Qemu-devel] [PATCH v4 0/8] xen: xen-domid-restrict improvements, Ian Jackson, 2017/10/09
- [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Ian Jackson, 2017/10/09
- Re: [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Ian Jackson, 2017/10/09
- Re: [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Anthony PERARD, 2017/10/10
- Re: [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Ian Jackson, 2017/10/10
- Re: [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Ian Jackson, 2017/10/19
- Re: [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Anthony PERARD, 2017/10/19
- Re: [Qemu-devel] [PATCH 1/8] xen: link against xentoolcore, Ian Jackson, 2017/10/20
[Qemu-devel] [PATCH 2/8] xen: restrict: use xentoolcore_restrict_all, Ian Jackson, 2017/10/09