[lxc-devel] [lxc/master] Fix mount_entry_on_generic()
brauner on Github
lxc-bot at linuxcontainers.org
Tue Feb 2 21:16:31 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1310 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160202/b3b6100e/attachment.bin>
-------------- next part --------------
From d6425e8dee4ed048c27125bfb1994af0dc70b91d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at mailbox.org>
Date: Tue, 2 Feb 2016 22:13:07 +0100
Subject: [PATCH] Fix mount_entry_on_generic()
In mount_entry_on_generic() we dereferenced a NULL pointer whenever a container
without a rootfs was created. (Since mount_entry_on_systemfs() passes them with
NULL.) We have mount_entry_on_generic() check whether rootfs != NULL.
We also check whether rootfs != NULL in the functions ovl_mkdir() and
mount_entry_create_aufs_dirs() and bail immediately. Rationale: For overlay and
aufs lxc.mount.entry entries users give us absolute paths to e.g. workdir and
upperdir which we create for them. We currently use rootfs->path and the
lxcpath for the container to check that users give us a sane path to create
those directories under and refuse if they do not. If we want to allow overlay
mounts for containers without a rootfs they can easily be reworked.
Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>
---
src/lxc/bdev/lxcoverlay.c | 3 +++
src/lxc/conf.c | 55 ++++++++++++++++++-----------------------------
2 files changed, 24 insertions(+), 34 deletions(-)
diff --git a/src/lxc/bdev/lxcoverlay.c b/src/lxc/bdev/lxcoverlay.c
index b954f93..d18f062 100644
--- a/src/lxc/bdev/lxcoverlay.c
+++ b/src/lxc/bdev/lxcoverlay.c
@@ -489,6 +489,9 @@ int ovl_mkdir(const struct mntent *mntent, const struct lxc_rootfs *rootfs,
size_t len = 0;
size_t rootfslen = 0;
+ /* Since we use all of these to check whether the user has given us a
+ * sane absolute path to create the directories needed for overlay
+ * lxc.mount.entry entries we consider any of these missing fatal. */
if (!rootfs || !rootfs->path || !lxc_name || !lxc_path)
goto err;
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index e3cf447..527cefb 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -858,15 +858,10 @@ static int setup_dev_symlinks(const struct lxc_rootfs *rootfs)
int ret,i;
struct stat s;
- /* rootfs struct will be empty when container is created without rootfs. */
- char *rootfs_path = NULL;
- if (rootfs && rootfs->path)
- rootfs_path = rootfs->mount;
-
for (i = 0; i < sizeof(dev_symlinks) / sizeof(dev_symlinks[0]); i++) {
const struct dev_symlinks *d = &dev_symlinks[i];
- ret = snprintf(path, sizeof(path), "%s/dev/%s", rootfs_path ? rootfs_path : "", d->name);
+ ret = snprintf(path, sizeof(path), "%s/dev/%s", rootfs->path ? rootfs->mount : "", d->name);
if (ret < 0 || ret >= MAXPATHLEN)
return -1;
@@ -1069,18 +1064,13 @@ static int mount_autodev(const char *name, const struct lxc_rootfs *rootfs, cons
size_t clen;
char *path;
- /* rootfs struct will be empty when container is created without rootfs. */
- char *rootfs_path = NULL;
- if (rootfs && rootfs->path)
- rootfs_path = rootfs->mount;
-
INFO("Mounting container /dev");
/* $(rootfs->mount) + "/dev/pts" + '\0' */
- clen = (rootfs_path ? strlen(rootfs_path) : 0) + 9;
+ clen = (rootfs->path ? strlen(rootfs->mount) : 0) + 9;
path = alloca(clen);
- ret = snprintf(path, clen, "%s/dev", rootfs_path ? rootfs_path : "");
+ ret = snprintf(path, clen, "%s/dev", rootfs->path ? rootfs->mount : "");
if (ret < 0 || ret >= clen)
return -1;
@@ -1090,16 +1080,15 @@ static int mount_autodev(const char *name, const struct lxc_rootfs *rootfs, cons
return 0;
}
- ret = safe_mount("none", path, "tmpfs", 0, "size=500000,mode=755",
- rootfs_path);
- if (ret != 0) {
+ if (safe_mount("none", path, "tmpfs", 0, "size=500000,mode=755",
+ rootfs->path ? rootfs->mount : NULL)) {
SYSERROR("Failed mounting tmpfs onto %s\n", path);
- return -1;
+ return false;
}
INFO("Mounted tmpfs onto %s", path);
- ret = snprintf(path, clen, "%s/dev/pts", rootfs_path ? rootfs_path : "");
+ ret = snprintf(path, clen, "%s/dev/pts", rootfs->path ? rootfs->mount : "");
if (ret < 0 || ret >= clen)
return -1;
@@ -1143,14 +1132,9 @@ static int fill_autodev(const struct lxc_rootfs *rootfs)
int i;
mode_t cmask;
- /* rootfs struct will be empty when container is created without rootfs. */
- char *rootfs_path = NULL;
- if (rootfs && rootfs->path)
- rootfs_path = rootfs->mount;
-
INFO("Creating initial consoles under container /dev");
- ret = snprintf(path, MAXPATHLEN, "%s/dev", rootfs_path ? rootfs_path : "");
+ ret = snprintf(path, MAXPATHLEN, "%s/dev", rootfs->path ? rootfs->mount : "");
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("Error calculating container /dev location");
return -1;
@@ -1163,7 +1147,7 @@ static int fill_autodev(const struct lxc_rootfs *rootfs)
cmask = umask(S_IXUSR | S_IXGRP | S_IXOTH);
for (i = 0; i < sizeof(lxc_devs) / sizeof(lxc_devs[0]); i++) {
const struct lxc_devs *d = &lxc_devs[i];
- ret = snprintf(path, MAXPATHLEN, "%s/dev/%s", rootfs_path ? rootfs_path : "", d->name);
+ ret = snprintf(path, MAXPATHLEN, "%s/dev/%s", rootfs->path ? rootfs->mount : "", d->name);
if (ret < 0 || ret >= MAXPATHLEN)
return -1;
ret = mknod(path, d->mode, makedev(d->maj, d->min));
@@ -1183,7 +1167,7 @@ static int fill_autodev(const struct lxc_rootfs *rootfs)
}
fclose(pathfile);
if (safe_mount(hostpath, path, 0, MS_BIND, NULL,
- rootfs_path ? rootfs_path : NULL) != 0) {
+ rootfs->path ? rootfs->mount : NULL) != 0) {
SYSERROR("Failed bind mounting device %s from host into container",
d->name);
return -1;
@@ -1759,6 +1743,9 @@ static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
size_t len = 0;
size_t rootfslen = 0;
+ /* Since we use all of these to check whether the user has given us a
+ * sane absolute path to create the directories needed for overlay
+ * lxc.mount.entry entries we consider any of these missing fatal. */
if (!rootfs || !rootfs->path || !lxc_name || !lxc_path)
goto err;
@@ -1802,7 +1789,6 @@ static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
return fret;
}
-
static int mount_entry_create_dir_file(const struct mntent *mntent,
const char* path, const struct lxc_rootfs *rootfs,
const char *lxc_name, const char *lxc_path)
@@ -1844,6 +1830,8 @@ static int mount_entry_create_dir_file(const struct mntent *mntent,
return ret;
}
+/* rootfs, lxc_name, and lxc_path can be NULL when the container is created
+ * without a rootfs. */
static inline int mount_entry_on_generic(struct mntent *mntent,
const char* path, const struct lxc_rootfs *rootfs,
const char *lxc_name, const char *lxc_path)
@@ -1853,6 +1841,10 @@ static inline int mount_entry_on_generic(struct mntent *mntent,
int ret;
bool optional = hasmntopt(mntent, "optional") != NULL;
+ char *rootfs_path = NULL;
+ if (rootfs && rootfs->path)
+ rootfs_path = rootfs->mount;
+
ret = mount_entry_create_dir_file(mntent, path, rootfs, lxc_name, lxc_path);
if (ret < 0)
@@ -1865,13 +1857,8 @@ static inline int mount_entry_on_generic(struct mntent *mntent,
return -1;
}
- /* rootfs struct will be empty when container is created without rootfs. */
- char *rootfs_path = NULL;
- if (rootfs && rootfs->path)
- rootfs_path = rootfs->mount;
-
ret = mount_entry(mntent->mnt_fsname, path, mntent->mnt_type, mntflags,
- mntdata, optional, rootfs_path);
+ mntdata, optional, rootfs_path);
free(mntdata);
return ret;
@@ -1879,7 +1866,7 @@ static inline int mount_entry_on_generic(struct mntent *mntent,
static inline int mount_entry_on_systemfs(struct mntent *mntent)
{
- return mount_entry_on_generic(mntent, mntent->mnt_dir, NULL, NULL, NULL);
+ return mount_entry_on_generic(mntent, mntent->mnt_dir, NULL, NULL, NULL);
}
static int mount_entry_on_absolute_rootfs(struct mntent *mntent,
More information about the lxc-devel
mailing list