Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
> -Original Message- > From: Anthony PERARD [mailto:anthony.per...@citrix.com] > Sent: 06 November 2018 11:11 > To: Paul Durrant > Cc: George Dunlap ; xen- > de...@lists.xenproject.org; Ian Jackson ; Wei Liu > > Subject: Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to > chroot > > On Tue, Nov 06, 2018 at 10:53:48AM +, Paul Durrant wrote: > > Ok. The trace backend is set at build time in tools/Makefile: > > > > if $$source/scripts/tracetool.py --check-backend --backend log ; > then \ > > enable_trace_backend='--enable-trace-backend=log'; > > elif $$source/scripts/tracetool.py --check-backend --backend > stderr ; then \ > > enable_trace_backend='--enable-trace-backend=stderr'; \ > > else \ > > enable_trace_backend='' ; \ > > fi ; \ > > > > and log appears to favoured. Is this still going to work with the > > chroot? We rely on the tracing in xen_platform to get log messages out > > of PV drivers. > > log vs stderr are just different names for the same thing. "stderr" > doesn't exist anymore in recent version of QEMU and as been renamed > "log". Oh yes, I'd forgotten... That's fine then :-) Paul > > -- > Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
On Tue, Nov 06, 2018 at 10:53:48AM +, Paul Durrant wrote: > Ok. The trace backend is set at build time in tools/Makefile: > > if $$source/scripts/tracetool.py --check-backend --backend log ; then > \ > enable_trace_backend='--enable-trace-backend=log'; > elif $$source/scripts/tracetool.py --check-backend --backend stderr ; > then \ > enable_trace_backend='--enable-trace-backend=stderr'; \ > else \ > enable_trace_backend='' ; \ > fi ; \ > > and log appears to favoured. Is this still going to work with the > chroot? We rely on the tracing in xen_platform to get log messages out > of PV drivers. log vs stderr are just different names for the same thing. "stderr" doesn't exist anymore in recent version of QEMU and as been renamed "log". -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
> -Original Message-
> From: George Dunlap [mailto:george.dun...@citrix.com]
> Sent: 06 November 2018 10:28
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Anthony Perard ; Ian Jackson
> ; Wei Liu
> Subject: Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to
> chroot
>
> On 11/06/2018 09:14 AM, Paul Durrant wrote:
> >> -Original Message-
> >> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On
> Behalf
> >> Of George Dunlap
> >> Sent: 05 November 2018 18:07
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Anthony Perard ; Ian Jackson
> >> ; Wei Liu ; George Dunlap
> >>
> >> Subject: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to
> chroot
> >>
> >> When dm_restrict is enabled, ask QEMU to chroot into an empty
> directory.
> >>
> >> * Create /var/run/qemu/root-domid (deleting the old one if it's there)
> >
> > This does not appear to match the code: the path should be
> /var/run/qemu-root- AFAICT
>
> Indeed, I forgot to update this. I can fix this up on check-in.
>
> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >> index 26eb16af34..ad3efcc783 100644
> >> --- a/tools/libxl/libxl_dm.c
> >> +++ b/tools/libxl/libxl_dm.c
> >> @@ -1410,9 +1410,48 @@ static int
> >> libxl__build_device_model_args_new(libxl__gc *gc,
> >> }
> >> }
> >>
> >> -if (libxl_defbool_val(b_info->dm_restrict))
> >> +if (libxl_defbool_val(b_info->dm_restrict)) {
> >> +char *chroot_dir = GCSPRINTF("%s/qemu-root-%d",
> >> + libxl__run_dir_path(),
> >> guest_domid);
> >> +int r;
> >> +
> >> flexarray_append(dm_args, "-xen-domid-restrict");
> >>
> >> +/*
> >> + * Run QEMU in a chroot at XEN_RUN_DIR/qemu-root-%d
> >
> > Maybe '' in the comment rather than '%d'?
>
> Maybe. :-)
>
> >> + *
> >> + * There is no library function to do the equivalent of `rm
> >> + * -rf`. However deprivileged QEMU in theory shouldn't be
> >> + * able to write any files, as the chroot would be owned by
> >> + * root, but it would be running as an unprivileged process.
> >> + * So in theory, old chroots should always be empty.
> >
> > How does logging work if QEMU can't write to the chroot? I assume we are
> relying on stderr? Does using syslog still work?
>
> Everything QEMU needs access to (including vnc sockets, qmp sockets, )
> must either be opened before the chroot happens, or passed to QEMU as an
> fd via qmp. In the case of logging, this happens through stderr; but if
> you search for 'chroot' in the design document you'll get a couple of
> examples of different issues that need to be addressed (including
> inserting a cd-rom and dealing with migration).
Ok. The trace backend is set at build time in tools/Makefile:
if $$source/scripts/tracetool.py --check-backend --backend log ; then \
enable_trace_backend='--enable-trace-backend=log';
elif $$source/scripts/tracetool.py --check-backend --backend stderr ;
then \
enable_trace_backend='--enable-trace-backend=stderr'; \
else \
enable_trace_backend='' ; \
fi ; \
and log appears to favoured. Is this still going to work with the chroot? We
rely on the tracing in xen_platform to get log messages out of PV drivers.
Paul
>
> -George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
On 11/06/2018 09:14 AM, Paul Durrant wrote:
>> -Original Message-
>> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
>> Of George Dunlap
>> Sent: 05 November 2018 18:07
>> To: xen-devel@lists.xenproject.org
>> Cc: Anthony Perard ; Ian Jackson
>> ; Wei Liu ; George Dunlap
>>
>> Subject: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
>>
>> When dm_restrict is enabled, ask QEMU to chroot into an empty directory.
>>
>> * Create /var/run/qemu/root-domid (deleting the old one if it's there)
>
> This does not appear to match the code: the path should be
> /var/run/qemu-root- AFAICT
Indeed, I forgot to update this. I can fix this up on check-in.
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 26eb16af34..ad3efcc783 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -1410,9 +1410,48 @@ static int
>> libxl__build_device_model_args_new(libxl__gc *gc,
>> }
>> }
>>
>> -if (libxl_defbool_val(b_info->dm_restrict))
>> +if (libxl_defbool_val(b_info->dm_restrict)) {
>> +char *chroot_dir = GCSPRINTF("%s/qemu-root-%d",
>> + libxl__run_dir_path(),
>> guest_domid);
>> +int r;
>> +
>> flexarray_append(dm_args, "-xen-domid-restrict");
>>
>> +/*
>> + * Run QEMU in a chroot at XEN_RUN_DIR/qemu-root-%d
>
> Maybe '' in the comment rather than '%d'?
Maybe. :-)
>> + *
>> + * There is no library function to do the equivalent of `rm
>> + * -rf`. However deprivileged QEMU in theory shouldn't be
>> + * able to write any files, as the chroot would be owned by
>> + * root, but it would be running as an unprivileged process.
>> + * So in theory, old chroots should always be empty.
>
> How does logging work if QEMU can't write to the chroot? I assume we are
> relying on stderr? Does using syslog still work?
Everything QEMU needs access to (including vnc sockets, qmp sockets, )
must either be opened before the chroot happens, or passed to QEMU as an
fd via qmp. In the case of logging, this happens through stderr; but if
you search for 'chroot' in the design document you'll get a couple of
examples of different issues that need to be addressed (including
inserting a cd-rom and dealing with migration).
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of George Dunlap
> Sent: 05 November 2018 18:07
> To: xen-devel@lists.xenproject.org
> Cc: Anthony Perard ; Ian Jackson
> ; Wei Liu ; George Dunlap
>
> Subject: [Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
>
> When dm_restrict is enabled, ask QEMU to chroot into an empty directory.
>
> * Create /var/run/qemu/root-domid (deleting the old one if it's there)
This does not appear to match the code: the path should be
/var/run/qemu-root- AFAICT
> * Pass the -chroot option to QEMU
>
> Rather than running `rm -rf` on the directory before creating it
> (since there is no library function to do this), simply rmdir the
> directory, relying on the fact that the previous QEMU instance, if
> properly restricted, shouldn't have been able to write anything
> anyway.
>
> Suggested-by: Ross Lagerwall
> Signed-off-by: George Dunlap
> Acked-by: Ian Jackson
> ---
> Changes since v2:
> - Style fixes
> - Testing moved to a different patch
>
> CC: Ian Jackson
> CC: Wei Liu
> CC: Anthony Perard
> ---
> docs/designs/qemu-deprivilege.md | 12 +-
> tools/libxl/libxl_dm.c | 41 +++-
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-
> deprivilege.md
> index 787ae1ac7c..039540 100644
> --- a/docs/designs/qemu-deprivilege.md
> +++ b/docs/designs/qemu-deprivilege.md
> @@ -61,12 +61,6 @@ source tree.)
>
> '''Testing status''': Tested
>
> -# Restrictions / improvements still to do
> -
> -This lists potential restrictions still to do. It is meant to be
> -listed in order of ease of implementation, with low-hanging fruit
> -first.
> -
> ## Chroot
>
> '''Description''': Qemu runs in its own chroot, such that even if it
> @@ -84,6 +78,12 @@ Then adds the following to the qemu command-line:
>
> '''Tested''': Not tested
>
> +## Restrictions / improvements still to do
> +
> +This lists potential restrictions still to do. It is meant to be
> +listed in order of ease of implementation, with low-hanging fruit
> +first.
> +
> ## Namespaces for unused functionality (Linux only)
>
> '''Description''': QEMU doesn't use the functionality associated with
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 26eb16af34..ad3efcc783 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1410,9 +1410,48 @@ static int
> libxl__build_device_model_args_new(libxl__gc *gc,
> }
> }
>
> -if (libxl_defbool_val(b_info->dm_restrict))
> +if (libxl_defbool_val(b_info->dm_restrict)) {
> +char *chroot_dir = GCSPRINTF("%s/qemu-root-%d",
> + libxl__run_dir_path(),
> guest_domid);
> +int r;
> +
> flexarray_append(dm_args, "-xen-domid-restrict");
>
> +/*
> + * Run QEMU in a chroot at XEN_RUN_DIR/qemu-root-%d
Maybe '' in the comment rather than '%d'?
> + *
> + * There is no library function to do the equivalent of `rm
> + * -rf`. However deprivileged QEMU in theory shouldn't be
> + * able to write any files, as the chroot would be owned by
> + * root, but it would be running as an unprivileged process.
> + * So in theory, old chroots should always be empty.
How does logging work if QEMU can't write to the chroot? I assume we are
relying on stderr? Does using syslog still work?
Paul
> + *
> + * rmdir the directory before attempting to create
> + * it; if it returns anything other than ENOENT, fail domain
> + * creation.
> + */
> +r = rmdir(chroot_dir);
> +if (r != 0 && errno != ENOENT) {
> +LOGED(ERROR, guest_domid,
> + "failed to remove existing chroot dir %s", chroot_dir);
> +return ERROR_FAIL;
> +}
> +
> +for (;;) {
> +r = mkdir(chroot_dir, );
> +if (!r)
> +break;
> +if (errno == EINTR) continue;
> +LOGED(ERROR, guest_domid,
> + "failed to create chroot dir %s", chroot_dir);
> +return ERROR_FAIL;
> +}
> +
> +/* Add "-chroot [dir]" to command-line */
> +flexarray_append(dm_args, "-chroot");
> +flexarray_append(dm_args, chroot_dir);
> +}
> +
> if (state->saved_state) {
> /* This file descriptor is meant to be used by QEMU */
> *dm_state_fd = open(state->saved_state, O_RDONLY);
> --
> 2.19.1
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 3/6] tools/dm_restrict: Ask QEMU to chroot
When dm_restrict is enabled, ask QEMU to chroot into an empty directory.
* Create /var/run/qemu/root-domid (deleting the old one if it's there)
* Pass the -chroot option to QEMU
Rather than running `rm -rf` on the directory before creating it
(since there is no library function to do this), simply rmdir the
directory, relying on the fact that the previous QEMU instance, if
properly restricted, shouldn't have been able to write anything
anyway.
Suggested-by: Ross Lagerwall
Signed-off-by: George Dunlap
Acked-by: Ian Jackson
---
Changes since v2:
- Style fixes
- Testing moved to a different patch
CC: Ian Jackson
CC: Wei Liu
CC: Anthony Perard
---
docs/designs/qemu-deprivilege.md | 12 +-
tools/libxl/libxl_dm.c | 41 +++-
2 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/docs/designs/qemu-deprivilege.md b/docs/designs/qemu-deprivilege.md
index 787ae1ac7c..039540 100644
--- a/docs/designs/qemu-deprivilege.md
+++ b/docs/designs/qemu-deprivilege.md
@@ -61,12 +61,6 @@ source tree.)
'''Testing status''': Tested
-# Restrictions / improvements still to do
-
-This lists potential restrictions still to do. It is meant to be
-listed in order of ease of implementation, with low-hanging fruit
-first.
-
## Chroot
'''Description''': Qemu runs in its own chroot, such that even if it
@@ -84,6 +78,12 @@ Then adds the following to the qemu command-line:
'''Tested''': Not tested
+## Restrictions / improvements still to do
+
+This lists potential restrictions still to do. It is meant to be
+listed in order of ease of implementation, with low-hanging fruit
+first.
+
## Namespaces for unused functionality (Linux only)
'''Description''': QEMU doesn't use the functionality associated with
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 26eb16af34..ad3efcc783 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1410,9 +1410,48 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
}
}
-if (libxl_defbool_val(b_info->dm_restrict))
+if (libxl_defbool_val(b_info->dm_restrict)) {
+char *chroot_dir = GCSPRINTF("%s/qemu-root-%d",
+ libxl__run_dir_path(), guest_domid);
+int r;
+
flexarray_append(dm_args, "-xen-domid-restrict");
+/*
+ * Run QEMU in a chroot at XEN_RUN_DIR/qemu-root-%d
+ *
+ * There is no library function to do the equivalent of `rm
+ * -rf`. However deprivileged QEMU in theory shouldn't be
+ * able to write any files, as the chroot would be owned by
+ * root, but it would be running as an unprivileged process.
+ * So in theory, old chroots should always be empty.
+ *
+ * rmdir the directory before attempting to create
+ * it; if it returns anything other than ENOENT, fail domain
+ * creation.
+ */
+r = rmdir(chroot_dir);
+if (r != 0 && errno != ENOENT) {
+LOGED(ERROR, guest_domid,
+ "failed to remove existing chroot dir %s", chroot_dir);
+return ERROR_FAIL;
+}
+
+for (;;) {
+r = mkdir(chroot_dir, );
+if (!r)
+break;
+if (errno == EINTR) continue;
+LOGED(ERROR, guest_domid,
+ "failed to create chroot dir %s", chroot_dir);
+return ERROR_FAIL;
+}
+
+/* Add "-chroot [dir]" to command-line */
+flexarray_append(dm_args, "-chroot");
+flexarray_append(dm_args, chroot_dir);
+}
+
if (state->saved_state) {
/* This file descriptor is meant to be used by QEMU */
*dm_state_fd = open(state->saved_state, O_RDONLY);
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
