diff mbox

[21/28] libxl: dm user: Reject attempts to set user!=root with qemu trad

Message ID 1450809903-3393-22-git-send-email-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ian Jackson Dec. 22, 2015, 6:44 p.m. UTC
Previously this option would be silently ignored, which is a potential
security problem (introduced in 84f2fd1b "run QEMU as non-root" in
xen-unstable only).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
v6: New patch.
---
 tools/libxl/libxl_dm.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Ian Campbell Jan. 7, 2016, 5:20 p.m. UTC | #1
On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> Previously this option would be silently ignored, which is a potential
> security problem (introduced in 84f2fd1b "run QEMU as non-root" in
> xen-unstable only).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>
(could/should go in now despite RFC-ness of the series as a whole, assuming
it is as independent as it looks, we really don't want to forget this for
4.7 if the other 27 patches take longer to land)


> ---
> v6: New patch.
> ---
>  tools/libxl/libxl_dm.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 886ed9c..8232981 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -415,6 +415,14 @@ static int
> libxl__build_device_model_args_old(libxl__gc *gc,
>      dm_args = flexarray_make(gc, 16, 1);
>      dm_envs = flexarray_make(gc, 16, 1);
>  
> +    if (b_info->device_model_user && /* default is NULL if stubdom */
> +        strcmp(b_info->device_model_user,"root")) {
> +        LOG(ERROR,
> + "device_model_user != root (%s) not supported by qemu-xen-traditional",
> +            b_info->device_model_user);
> +        return ERROR_INVAL;
> +    }
> +
>      flexarray_vappend(dm_args, dm,
>                        "-d", GCSPRINTF("%d", domid), NULL);
>
Ian Jackson Jan. 8, 2016, 12:16 p.m. UTC | #2
Ian Campbell writes ("Re: [PATCH 21/28] libxl: dm user: Reject attempts to set user!=root with qemu trad"):
> On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> > Previously this option would be silently ignored, which is a potential
> > security problem (introduced in 84f2fd1b "run QEMU as non-root" in
> > xen-unstable only).
> > 
> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> (could/should go in now despite RFC-ness of the series as a whole, assuming
> it is as independent as it looks, we really don't want to forget this for
> 4.7 if the other 27 patches take longer to land)

Well, it moved easily to earlier in the series, so I have done that.
But there are other things wrong with 84f2fd1b which are sorted out
here, too.  I think if this series doesn't make 4.7 we may need to
revert 84f2fd1b, or at least consider other parts of this series to
cherry pick.

Thanks,
Ian.
Ian Campbell Jan. 8, 2016, 12:23 p.m. UTC | #3
On Fri, 2016-01-08 at 12:16 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 21/28] libxl: dm user: Reject attempts
> to set user!=root with qemu trad"):
> > On Tue, 2015-12-22 at 18:44 +0000, Ian Jackson wrote:
> > > Previously this option would be silently ignored, which is a
> > > potential
> > > security problem (introduced in 84f2fd1b "run QEMU as non-root" in
> > > xen-unstable only).
> > > 
> > > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > > CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > (could/should go in now despite RFC-ness of the series as a whole,
> > assuming
> > it is as independent as it looks, we really don't want to forget this
> > for
> > 4.7 if the other 27 patches take longer to land)
> 
> Well, it moved easily to earlier in the series, so I have done that.
> But there are other things wrong with 84f2fd1b which are sorted out
> here, too.  I think if this series doesn't make 4.7 we may need to
> revert 84f2fd1b, or at least consider other parts of this series to
> cherry pick.

Makes sense, I assume you'll either propose that revert of a suitable list
of cherry-picks if/when the time comes.

Ian.
diff mbox

Patch

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 886ed9c..8232981 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -415,6 +415,14 @@  static int libxl__build_device_model_args_old(libxl__gc *gc,
     dm_args = flexarray_make(gc, 16, 1);
     dm_envs = flexarray_make(gc, 16, 1);
 
+    if (b_info->device_model_user && /* default is NULL if stubdom */
+        strcmp(b_info->device_model_user,"root")) {
+        LOG(ERROR,
+ "device_model_user != root (%s) not supported by qemu-xen-traditional",
+            b_info->device_model_user);
+        return ERROR_INVAL;
+    }
+
     flexarray_vappend(dm_args, dm,
                       "-d", GCSPRINTF("%d", domid), NULL);