Unverified Commit cd2ca8a1 by Rachid Koucha Committed by Christian Brauner

lxccontainer: do not display if missing privileges

lxc-ls without root privileges on privileged containers should not display information. In lxc_container_new(), ongoing_create()'s result is not checked for all possible returned values. Hence, an unprivileged user can send command messages to the container's monitor. For example: $ lxc-ls -P /.../tests -f NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED ctr - 0 - - - false $ sudo lxc-ls -P /.../tests -f NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED ctr RUNNING 0 - 10.0.3.51 - false After this change: $ lxc-ls -P /.../tests -f <-------- No more display without root privileges $ sudo lxc-ls -P /.../tests -f NAME STATE AUTOSTART GROUPS IPV4 IPV6 UNPRIVILEGED ctr RUNNING 0 - 10.0.3.37 - false $ Signed-off-by: 's avatarRachid Koucha <rachid.koucha@gmail.com> Signed-off-by: 's avatarChristian Brauner <christian.brauner@ubuntu.com>
parent 46dde527
...@@ -132,7 +132,8 @@ static bool config_file_exists(const char *lxcpath, const char *cname) ...@@ -132,7 +132,8 @@ static bool config_file_exists(const char *lxcpath, const char *cname)
return file_exists(fname); return file_exists(fname);
} }
/* A few functions to help detect when a container creation failed. If a /*
* A few functions to help detect when a container creation failed. If a
* container creation was killed partway through, then trying to actually start * container creation was killed partway through, then trying to actually start
* that container could harm the host. We detect this by creating a 'partial' * that container could harm the host. We detect this by creating a 'partial'
* file under the container directory, and keeping an advisory lock. When * file under the container directory, and keeping an advisory lock. When
...@@ -140,30 +141,39 @@ static bool config_file_exists(const char *lxcpath, const char *cname) ...@@ -140,30 +141,39 @@ static bool config_file_exists(const char *lxcpath, const char *cname)
* start a container, if we find that file, without a flock, we remove the * start a container, if we find that file, without a flock, we remove the
* container. * container.
*/ */
enum {
LXC_CREATE_FAILED = -1,
LXC_CREATE_SUCCESS = 0,
LXC_CREATE_ONGOING = 1,
LXC_CREATE_INCOMPLETE = 2,
};
static int ongoing_create(struct lxc_container *c) static int ongoing_create(struct lxc_container *c)
{ {
__do_close_prot_errno int fd = -EBADF;
__do_free char *path = NULL; __do_free char *path = NULL;
int fd, ret;
size_t len;
struct flock lk = {0}; struct flock lk = {0};
int ret;
size_t len;
len = strlen(c->config_path) + strlen(c->name) + 10; len = strlen(c->config_path) + strlen(c->name) + 10;
path = must_realloc(NULL, len); path = must_realloc(NULL, len);
ret = snprintf(path, len, "%s/%s/partial", c->config_path, c->name); ret = snprintf(path, len, "%s/%s/partial", c->config_path, c->name);
if (ret < 0 || (size_t)ret >= len) if (ret < 0 || (size_t)ret >= len)
return -1; return LXC_CREATE_FAILED;
fd = open(path, O_RDWR | O_CLOEXEC); fd = open(path, O_RDWR | O_CLOEXEC);
if (fd < 0) { if (fd < 0) {
if (errno != ENOENT) if (errno != ENOENT)
return -1; return LXC_CREATE_FAILED;
return 0; return LXC_CREATE_SUCCESS;
} }
lk.l_type = F_WRLCK; lk.l_type = F_WRLCK;
lk.l_whence = SEEK_SET; lk.l_whence = SEEK_SET;
/* F_OFD_GETLK requires that l_pid be set to 0 otherwise the kernel /*
* F_OFD_GETLK requires that l_pid be set to 0 otherwise the kernel
* will EINVAL us. * will EINVAL us.
*/ */
lk.l_pid = 0; lk.l_pid = 0;
...@@ -175,15 +185,13 @@ static int ongoing_create(struct lxc_container *c) ...@@ -175,15 +185,13 @@ static int ongoing_create(struct lxc_container *c)
ret = 0; ret = 0;
} }
close(fd);
/* F_OFD_GETLK will not send us back a pid so don't check it. */ /* F_OFD_GETLK will not send us back a pid so don't check it. */
if (ret == 0) if (ret == 0)
/* Create is still ongoing. */ /* Create is still ongoing. */
return 1; return LXC_CREATE_ONGOING;
/* Create completed but partial is still there. */ /* Create completed but partial is still there. */
return 2; return LXC_CREATE_INCOMPLETE;
} }
static int create_partial(struct lxc_container *c) static int create_partial(struct lxc_container *c)
...@@ -869,13 +877,14 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a ...@@ -869,13 +877,14 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
return false; return false;
ret = ongoing_create(c); ret = ongoing_create(c);
if (ret < 0) { switch (ret) {
case LXC_CREATE_FAILED:
ERROR("Failed checking for incomplete container creation"); ERROR("Failed checking for incomplete container creation");
return false; return false;
} else if (ret == 1) { case LXC_CREATE_ONGOING:
ERROR("Ongoing container creation detected"); ERROR("Ongoing container creation detected");
return false; return false;
} else if (ret == 2) { case LXC_CREATE_INCOMPLETE:
ERROR("Failed to create container"); ERROR("Failed to create container");
do_lxcapi_destroy(c); do_lxcapi_destroy(c);
return false; return false;
...@@ -4974,6 +4983,7 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath ...@@ -4974,6 +4983,7 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
{ {
struct lxc_container *c; struct lxc_container *c;
size_t len; size_t len;
int rc;
if (!name) if (!name)
return NULL; return NULL;
...@@ -5027,10 +5037,24 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath ...@@ -5027,10 +5037,24 @@ struct lxc_container *lxc_container_new(const char *name, const char *configpath
goto err; goto err;
} }
if (ongoing_create(c) == 2) { rc = ongoing_create(c);
ERROR("Failed to complete container creation for %s", c->name); switch (rc) {
case LXC_CREATE_INCOMPLETE:
SYSERROR("Failed to complete container creation for %s", c->name);
container_destroy(c, NULL); container_destroy(c, NULL);
lxcapi_clear_config(c); lxcapi_clear_config(c);
break;
case LXC_CREATE_ONGOING:
/* container creation going on */
break;
case LXC_CREATE_FAILED:
/* container creation failed */
if (errno != EACCES && errno != EPERM) {
/* insufficient privileges */
SYSERROR("Failed checking for incomplete container %s creation", c->name);
goto err;
}
break;
} }
c->daemonize = true; c->daemonize = true;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment