[lxc-devel] [PATCH] fix autodev on SELinux enabled systems

Serge Hallyn serge.hallyn at ubuntu.com
Fri Feb 13 05:12:09 UTC 2015


Quoting Dwight Engen (dwight.engen at oracle.com):
> On Thu, 12 Feb 2015 23:39:48 +0000
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > commit 87da4ec3 changed autodev such that device nodes are created
> > > in a small tmpfs, rather than in a subdirectory of /dev. This
> > > causes a problem on an SELinux enabled host since the device nodes
> > > will have a context type of user_tmp_t. By labeling the tmpfs with
> > > device_t, udevd will assign the autodev created devices with the
> > > correct labels (ie. null_device_t for /dev/null).
> > > 
> > > The bad labels were causing things like dhclient to fail in the
> > > container since they couldn't access /dev/null.
> > > 
> > > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> > 
> > Thanks.  A few questions,
> > 
> > > ---
> > >  src/lxc/conf.c        | 10 ++++++++--
> > >  src/lxc/lsm/lsm.c     | 11 +++++++++++
> > >  src/lxc/lsm/lsm.h     |  3 +++
> > >  src/lxc/lsm/selinux.c | 19 ++++++++++++++++---
> > >  4 files changed, 38 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > index 2868708..9a826ce 100644
> > > --- a/src/lxc/conf.c
> > > +++ b/src/lxc/conf.c
> > > @@ -1132,7 +1132,7 @@ static int mount_autodev(const char *name,
> > > char *root, const char *lxcpath) {
> > >  	int ret;
> > >  	size_t clen;
> > > -	char *path;
> > > +	char *path,*mnt_opts;
> > >  
> > >  	INFO("Mounting /dev under %s", root);
> > >  
> > > @@ -1150,7 +1150,13 @@ static int mount_autodev(const char *name,
> > > char *root, const char *lxcpath) return 0;
> > >  	}
> > >  
> > > -	if (mount("none", path, "tmpfs", 0,
> > > "size=100000,mode=755")) {
> > > +	if (asprintf(&mnt_opts, "size=100000,mode=755%s",
> > 
> > Anyone know offhand - is asprintf available on android?
> 
> Not sure, but asprintf is already used in lxc_execute.c and lxc_start.c
> so I figured it was safe :)

Stéphane also says it's safe, so good enough :)

> > > +		     lsm_autodev_mount_opts()) < 0)
> > > +		return -1;
> > > +
> > > +	ret = mount("none", path, "tmpfs", 0, mnt_opts);
> > > +	free(mnt_opts);
> > > +	if (ret) {
> > >  		SYSERROR("Failed mounting tmpfs onto %s\n", path);
> > >  		return false;
> > >  	}
> > > diff --git a/src/lxc/lsm/lsm.c b/src/lxc/lsm/lsm.c
> > > index 79f837f..400e689 100644
> > > --- a/src/lxc/lsm/lsm.c
> > > +++ b/src/lxc/lsm/lsm.c
> > > @@ -95,4 +95,15 @@ int lsm_process_label_set(const char *label,
> > > struct lxc_conf *conf, return drv->process_label_set(label, conf,
> > > use_default, on_exec); }
> > >  
> > > +char *lsm_autodev_mount_opts(void)
> > > +{
> > > +	if (!drv) {
> > > +		ERROR("LSM driver not inited");
> > > +		return "";
> > > +	}
> > > +	if (drv->autodev_mount_opts)
> > > +		return drv->autodev_mount_opts();
> > > +	return "";
> > > +}
> > > +
> > >  #endif
> > > diff --git a/src/lxc/lsm/lsm.h b/src/lxc/lsm/lsm.h
> > > index d6a7aac..8739674 100644
> > > --- a/src/lxc/lsm/lsm.h
> > > +++ b/src/lxc/lsm/lsm.h
> > > @@ -35,6 +35,7 @@ struct lsm_drv {
> > >  	char *(*process_label_get)(pid_t pid);
> > >  	int   (*process_label_set)(const char *label, struct
> > > lxc_conf *conf, int use_default, int on_exec);
> > > +	char *(*autodev_mount_opts)(void);
> > >  };
> > >  
> > >  #if HAVE_APPARMOR || HAVE_SELINUX
> > > @@ -44,6 +45,7 @@ const char *lsm_name(void);
> > >  char       *lsm_process_label_get(pid_t pid);
> > >  int         lsm_process_label_set(const char *label, struct
> > > lxc_conf *conf, int use_default, int on_exec);
> > > +char       *lsm_autodev_mount_opts(void);
> > >  #else
> > >  static inline void        lsm_init(void) { }
> > >  static inline int         lsm_enabled(void) { return 0; }
> > > @@ -51,6 +53,7 @@ static inline const char *lsm_name(void) { return
> > > "none"; } static inline char       *lsm_process_label_get(pid_t
> > > pid) { return NULL; } static inline int
> > > lsm_process_label_set(const char *label, struct lxc_conf *conf, int
> > > use_default, int on_exec) { return 0; } +static inline char
> > > *lsm_autodev_mount_opts(void) { return ""; } #endif
> > >  
> > >  #endif
> > > diff --git a/src/lxc/lsm/selinux.c b/src/lxc/lsm/selinux.c
> > > index 46554d8..0bdcf90 100644
> > > --- a/src/lxc/lsm/selinux.c
> > > +++ b/src/lxc/lsm/selinux.c
> > > @@ -100,11 +100,24 @@ static int selinux_process_label_set(const
> > > char *inlabel, struct lxc_conf *conf, return 0;
> > >  }
> > >  
> > > +/*
> > > + * selinux_autodev_mount_opts: Set SELinux context for autodev's
> > > tmpfs
> > > + *
> > > + * Returns the mount option for setting context on filesystem
> > > mount. The tmpfs
> > > + * has to be labeled with device_t so that udevd will give the
> > > nodes
> > > + * that autodev creates in it the proper labels.
> > > + */
> > > +static char *selinux_autodev_mount_opts(void)
> > > +{
> > > +	return strdup(",fscontext=system_u:object_r:device_t:s0");
> > 
> > This allocation will be lost, won't it?
> 
> Yes good catch. This is because earlier I thought I needed a modifiable
> string. Looks like it can actually return a const char *.
> 
> > Also, what will this do to nested containers?  Can any user mount a
> > tmpfs with those contexts?
> 
> I don't have nested containers to test with here so I'm not sure.

Does it work with unprivileged containers?

> After further testing I noticed that this method doesn't work on
> my older OL6.x host, the autodev nodes end up as device_t just like the
> directory instead of their proper contexts so I'm not sure this is
> enough to wholly fix the problem. If you know of a better way to get
> the contexts set correctly I'd be happy to change the patch.

No, this is probably the best.  But I'd recommend that if the mount
with extraopts fails, we then try without the extraopts.

> > Is s0 always an ok category?
> 
> I think so, the full context is the same as that on regular /dev.
>  
> > > +}
> > > +
> > >  static struct lsm_drv selinux_drv = {
> > >  	.name = "SELinux",
> > > -	.enabled           = is_selinux_enabled,
> > > -	.process_label_get = selinux_process_label_get,
> > > -	.process_label_set = selinux_process_label_set,
> > > +	.enabled            = is_selinux_enabled,
> > > +	.process_label_get  = selinux_process_label_get,
> > > +	.process_label_set  = selinux_process_label_set,
> > > +	.autodev_mount_opts = selinux_autodev_mount_opts,
> > >  };
> > >  
> > >  struct lsm_drv *lsm_selinux_drv_init(void)
> > > -- 
> > > 1.9.3
> > > 
> > > _______________________________________________
> > > lxc-devel mailing list
> > > lxc-devel at lists.linuxcontainers.org
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel


More information about the lxc-devel mailing list