From: Ian Jackson <ian.jackson@eu.citrix.com> To: qemu-devel@nongnu.org Cc: Ross Lagerwall <ross.lagerwall@citrix.com>, Anthony PERARD <anthony.perard@citrix.com>, Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, xen-devel@lists.xenproject.org, Ian Jackson <ian.jackson@eu.citrix.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com> Subject: [Qemu-devel] [PATCH 05/16] xen: defer call to xen_restrict until just before os_setup_post Date: Thu, 19 Apr 2018 17:45:08 +0100 [thread overview] Message-ID: <1524156319-11465-6-git-send-email-ian.jackson@eu.citrix.com> (raw) In-Reply-To: <1524156319-11465-1-git-send-email-ian.jackson@eu.citrix.com> 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. Currently this does not work with migration, because when running as the Xen device model qemu needs to signal to the toolstack that it is ready. It currently does this using xenstore, and for incoming migration (but not for ordinary startup) that happens after os_setup_post. It is correct that this happens late: we want the incoming migration stream to be processed by a restricted qemu. The fix for this will be to do the startup notification a different way, without using xenstore. (QMP is probably a reasonable choice.) So for now this restriction feature cannot be used in conjunction with migration. (Note that this is not a regression in this patch, because previously the -xen-restrict-domid call was, in fact, simply ineffective!) We will revisit this in the Xen 4.11 release cycle. Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86) CC: Richard Henderson <rth@twiddle.net> (maintainer:X86) CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86) CC: Michael S. Tsirkin <mst@redhat.com> (supporter:PC) --- v7: Use new AccelClass setup_post hook, rather than ad-hoc call in vl.c. v5: Discuss problems with migration startup notification in the commit message. v3: Do xen_setup_post just before, not just after, os_setup_post, to improve interaction with chroot. Thanks to Ross Lagerwall. Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> --- hw/i386/xen/xen-hvm.c | 8 -------- hw/xen/xen-common.c | 14 ++++++++++++++ stubs/xen-hvm.c | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index f24b7d4..9c3b6b3 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -1254,14 +1254,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory) goto err; } - if (xen_domid_restrict) { - rc = xen_restrict(xen_domid); - if (rc < 0) { - error_report("failed to restrict: error %d", errno); - goto err; - } - } - xen_create_ioreq_server(xen_domid, &state->ioservid); state->exit.notify = xen_exit_notifier; diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c index 83099dd..454777c 100644 --- a/hw/xen/xen-common.c +++ b/hw/xen/xen-common.c @@ -117,6 +117,19 @@ static void xen_change_state_handler(void *opaque, int running, } } +static void xen_setup_post(MachineState *ms, AccelState *accel) +{ + int rc; + + if (xen_domid_restrict) { + rc = xen_restrict(xen_domid); + if (rc < 0) { + perror("xen: failed to restrict"); + exit(1); + } + } +} + static int xen_init(MachineState *ms) { xen_xc = xc_interface_open(0, 0, 0); @@ -165,6 +178,7 @@ static void xen_accel_class_init(ObjectClass *oc, void *data) AccelClass *ac = ACCEL_CLASS(oc); ac->name = "Xen"; ac->init_machine = xen_init; + ac->setup_post = xen_setup_post; ac->allowed = &xen_allowed; ac->global_props = xen_compat_props; } diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c index 0067bcc..7787ea2 100644 --- a/stubs/xen-hvm.c +++ b/stubs/xen-hvm.c @@ -13,6 +13,7 @@ #include "hw/xen/xen.h" #include "exec/memory.h" #include "qapi/qapi-commands-misc.h" +#include "sysemu/sysemu.h" int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) { -- 2.1.4
WARNING: multiple messages have this Message-ID (diff)
From: Ian Jackson <ian.jackson@eu.citrix.com> To: qemu-devel@nongnu.org Cc: Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Ross Lagerwall <ross.lagerwall@citrix.com>, Paolo Bonzini <pbonzini@redhat.com>, Anthony PERARD <anthony.perard@citrix.com>, xen-devel@lists.xenproject.org, Richard Henderson <rth@twiddle.net> Subject: [PATCH 05/16] xen: defer call to xen_restrict until just before os_setup_post Date: Thu, 19 Apr 2018 17:45:08 +0100 [thread overview] Message-ID: <1524156319-11465-6-git-send-email-ian.jackson@eu.citrix.com> (raw) In-Reply-To: <1524156319-11465-1-git-send-email-ian.jackson@eu.citrix.com> 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. Currently this does not work with migration, because when running as the Xen device model qemu needs to signal to the toolstack that it is ready. It currently does this using xenstore, and for incoming migration (but not for ordinary startup) that happens after os_setup_post. It is correct that this happens late: we want the incoming migration stream to be processed by a restricted qemu. The fix for this will be to do the startup notification a different way, without using xenstore. (QMP is probably a reasonable choice.) So for now this restriction feature cannot be used in conjunction with migration. (Note that this is not a regression in this patch, because previously the -xen-restrict-domid call was, in fact, simply ineffective!) We will revisit this in the Xen 4.11 release cycle. Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> CC: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86) CC: Richard Henderson <rth@twiddle.net> (maintainer:X86) CC: Eduardo Habkost <ehabkost@redhat.com> (maintainer:X86) CC: Michael S. Tsirkin <mst@redhat.com> (supporter:PC) --- v7: Use new AccelClass setup_post hook, rather than ad-hoc call in vl.c. v5: Discuss problems with migration startup notification in the commit message. v3: Do xen_setup_post just before, not just after, os_setup_post, to improve interaction with chroot. Thanks to Ross Lagerwall. Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> --- hw/i386/xen/xen-hvm.c | 8 -------- hw/xen/xen-common.c | 14 ++++++++++++++ stubs/xen-hvm.c | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index f24b7d4..9c3b6b3 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -1254,14 +1254,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory) goto err; } - if (xen_domid_restrict) { - rc = xen_restrict(xen_domid); - if (rc < 0) { - error_report("failed to restrict: error %d", errno); - goto err; - } - } - xen_create_ioreq_server(xen_domid, &state->ioservid); state->exit.notify = xen_exit_notifier; diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c index 83099dd..454777c 100644 --- a/hw/xen/xen-common.c +++ b/hw/xen/xen-common.c @@ -117,6 +117,19 @@ static void xen_change_state_handler(void *opaque, int running, } } +static void xen_setup_post(MachineState *ms, AccelState *accel) +{ + int rc; + + if (xen_domid_restrict) { + rc = xen_restrict(xen_domid); + if (rc < 0) { + perror("xen: failed to restrict"); + exit(1); + } + } +} + static int xen_init(MachineState *ms) { xen_xc = xc_interface_open(0, 0, 0); @@ -165,6 +178,7 @@ static void xen_accel_class_init(ObjectClass *oc, void *data) AccelClass *ac = ACCEL_CLASS(oc); ac->name = "Xen"; ac->init_machine = xen_init; + ac->setup_post = xen_setup_post; ac->allowed = &xen_allowed; ac->global_props = xen_compat_props; } diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c index 0067bcc..7787ea2 100644 --- a/stubs/xen-hvm.c +++ b/stubs/xen-hvm.c @@ -13,6 +13,7 @@ #include "hw/xen/xen.h" #include "exec/memory.h" #include "qapi/qapi-commands-misc.h" +#include "sysemu/sysemu.h" int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) { -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2018-04-19 16:45 UTC|newest] Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-19 16:45 [Qemu-devel] [PATCH v7 00/16] xen: xen-domid-restrict improvements Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 01/16] checkpatch: Add xendevicemodel_handle to the list of types Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 02/16] AccelClass: Introduce accel_setup_post Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 03/16] xen: link against xentoolcore Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-23 15:05 ` [Qemu-devel] " Anthony PERARD 2018-04-23 15:05 ` Anthony PERARD 2018-04-24 14:28 ` [Qemu-devel] " Ian Jackson 2018-04-24 14:28 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 04/16] xen: restrict: use xentoolcore_restrict_all Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` Ian Jackson [this message] 2018-04-19 16:45 ` [PATCH 05/16] xen: defer call to xen_restrict until just before os_setup_post Ian Jackson 2018-04-23 14:28 ` [Qemu-devel] " Anthony PERARD 2018-04-23 14:28 ` Anthony PERARD 2018-04-23 15:58 ` [Qemu-devel] " Anthony PERARD 2018-04-23 15:58 ` Anthony PERARD 2018-04-24 15:08 ` Ian Jackson 2018-04-24 15:08 ` [Qemu-devel] " Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 06/16] xen: destroy_hvm_domain: Move reason into a variable Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 07/16] xen: move xc_interface compatibility fallback further up the file Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 08/16] xen: destroy_hvm_domain: Try xendevicemodel_shutdown Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 09/16] os-posix: cleanup: Replace fprintfs with error_report in change_process_uid Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 10/16] os-posix: Provide new -runas <uid>:<gid> facility Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-20 12:56 ` [Qemu-devel] " Markus Armbruster 2018-04-20 12:56 ` Markus Armbruster 2018-04-19 16:45 ` [Qemu-devel] [PATCH 11/16] xen: Use newly added dmops for mapping VGA memory Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 12/16] xen: Remove now-obsolete xen_xc_domain_add_to_physmap Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-23 15:38 ` [Qemu-devel] " Anthony PERARD 2018-04-23 15:38 ` Anthony PERARD 2018-04-19 16:45 ` [Qemu-devel] [PATCH 13/16] xen: Expect xenstore write to fail when restricted Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 14/16] os-posix: cleanup: Replace fprintf with error_report in remaining call sites Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 20:25 ` [Qemu-devel] " Philippe Mathieu-Daudé 2018-04-19 20:25 ` Philippe Mathieu-Daudé 2018-04-20 10:19 ` Ian Jackson 2018-04-20 10:19 ` Ian Jackson 2018-04-19 16:45 ` [Qemu-devel] [PATCH 15/16] os-posix: cleanup: Replace perror with error_report Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-19 20:31 ` [Qemu-devel] " Philippe Mathieu-Daudé 2018-04-19 20:31 ` Philippe Mathieu-Daudé 2018-04-24 14:53 ` Ian Jackson 2018-04-24 14:53 ` Ian Jackson 2018-04-24 15:18 ` Daniel P. Berrangé 2018-04-24 15:18 ` Daniel P. Berrangé 2018-04-24 15:40 ` Eric Blake 2018-04-24 15:40 ` Eric Blake 2018-04-24 15:43 ` Eric Blake 2018-04-24 15:43 ` Eric Blake 2018-04-24 15:54 ` Daniel P. Berrangé 2018-04-24 15:54 ` Daniel P. Berrangé 2018-04-24 16:20 ` Markus Armbruster 2018-04-24 16:20 ` Markus Armbruster 2018-04-19 16:45 ` [Qemu-devel] [PATCH 16/16] configure: do_compiler: Dump some extra info under bash Ian Jackson 2018-04-19 16:45 ` Ian Jackson 2018-04-23 16:21 ` [Qemu-devel] " Anthony PERARD 2018-04-23 16:21 ` Anthony PERARD 2018-04-23 16:38 ` [Qemu-devel] " Daniel P. Berrangé 2018-04-23 17:12 ` Ian Jackson 2018-04-23 17:12 ` Ian Jackson 2018-04-23 16:38 ` Daniel P. Berrangé 2018-04-23 16:38 ` Ian Jackson 2018-04-23 16:38 ` Ian Jackson 2018-04-23 20:28 ` [Qemu-devel] " Eric Blake 2018-04-23 20:28 ` Eric Blake 2018-04-24 15:05 ` Ian Jackson 2018-04-24 15:05 ` Ian Jackson 2018-04-19 17:03 ` [Qemu-devel] [PATCH v7 00/16] xen: xen-domid-restrict improvements no-reply 2018-04-19 17:03 ` no-reply 2018-04-24 17:58 [Qemu-devel] [PATCH v8 " Ian Jackson 2018-04-24 17:58 ` [Qemu-devel] [PATCH 05/16] xen: defer call to xen_restrict until just before os_setup_post Ian Jackson
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=1524156319-11465-6-git-send-email-ian.jackson@eu.citrix.com \ --to=ian.jackson@eu.citrix.com \ --cc=anthony.perard@citrix.com \ --cc=ehabkost@redhat.com \ --cc=jgross@suse.com \ --cc=mst@redhat.com \ --cc=pbonzini@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=ross.lagerwall@citrix.com \ --cc=rth@twiddle.net \ --cc=sstabellini@kernel.org \ --cc=xen-devel@lists.xenproject.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.