[lxc-devel] [lxc/master] Fix NULL-ptr derefs for container without rootfs
brauner on Github
lxc-bot at linuxcontainers.org
Tue Feb 2 13:51:58 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 903 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160202/d0cf8ce7/attachment.bin>
-------------- next part --------------
From 1ec0e8e3fd766ff77c6128f384772f407f9b75da Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at mailbox.org>
Date: Tue, 2 Feb 2016 14:43:33 +0100
Subject: [PATCH] Fix NULL-ptr derefs for container without rootfs
Since we allow containers to be created without a rootfs most checks in conf.c
are not sane anymore. Instead of just checking if rootfs->path != NULL we need
to check whether rootfs != NULL.
Minor fixes:
- Have mount_autodev() always return -1 on failure: mount_autodev() returns 0
on success and -1 on failure. But when the return value of safe_mount() was
checked in mount_autodev() we returned false (instead of -1) which caused
mount_autodev() to return 0 (success) instead of the correct -1 (failure).
Signed-off-by: Christian Brauner <christian.brauner at mailbox.org>
---
src/lxc/bdev/lxcoverlay.c | 2 +-
src/lxc/conf.c | 46 +++++++++++++++++++++++++++++++++-------------
2 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/src/lxc/bdev/lxcoverlay.c b/src/lxc/bdev/lxcoverlay.c
index 22290cf..b954f93 100644
--- a/src/lxc/bdev/lxcoverlay.c
+++ b/src/lxc/bdev/lxcoverlay.c
@@ -489,7 +489,7 @@ int ovl_mkdir(const struct mntent *mntent, const struct lxc_rootfs *rootfs,
size_t len = 0;
size_t rootfslen = 0;
- if (!rootfs->path || !lxc_name || !lxc_path)
+ if (!rootfs || !rootfs->path || !lxc_name || !lxc_path)
goto err;
opts = lxc_string_split(mntent->mnt_opts, ',');
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 7f09542..e3cf447 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -858,10 +858,15 @@ 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->mount : "", d->name);
+ ret = snprintf(path, sizeof(path), "%s/dev/%s", rootfs_path ? rootfs_path : "", d->name);
if (ret < 0 || ret >= MAXPATHLEN)
return -1;
@@ -1064,13 +1069,18 @@ 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->mount) : 0) + 9;
+ clen = (rootfs_path ? strlen(rootfs_path) : 0) + 9;
path = alloca(clen);
- ret = snprintf(path, clen, "%s/dev", rootfs->path ? rootfs->mount : "");
+ ret = snprintf(path, clen, "%s/dev", rootfs_path ? rootfs_path : "");
if (ret < 0 || ret >= clen)
return -1;
@@ -1080,15 +1090,16 @@ static int mount_autodev(const char *name, const struct lxc_rootfs *rootfs, cons
return 0;
}
- if (safe_mount("none", path, "tmpfs", 0, "size=500000,mode=755",
- rootfs->path ? rootfs->mount : NULL)) {
+ ret = safe_mount("none", path, "tmpfs", 0, "size=500000,mode=755",
+ rootfs_path);
+ if (ret != 0) {
SYSERROR("Failed mounting tmpfs onto %s\n", path);
- return false;
+ return -1;
}
INFO("Mounted tmpfs onto %s", path);
- ret = snprintf(path, clen, "%s/dev/pts", rootfs->path ? rootfs->mount : "");
+ ret = snprintf(path, clen, "%s/dev/pts", rootfs_path ? rootfs_path : "");
if (ret < 0 || ret >= clen)
return -1;
@@ -1132,9 +1143,14 @@ 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->mount : "");
+ ret = snprintf(path, MAXPATHLEN, "%s/dev", rootfs_path ? rootfs_path : "");
if (ret < 0 || ret >= MAXPATHLEN) {
ERROR("Error calculating container /dev location");
return -1;
@@ -1147,7 +1163,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->mount : "", d->name);
+ ret = snprintf(path, MAXPATHLEN, "%s/dev/%s", rootfs_path ? rootfs_path : "", d->name);
if (ret < 0 || ret >= MAXPATHLEN)
return -1;
ret = mknod(path, d->mode, makedev(d->maj, d->min));
@@ -1167,7 +1183,7 @@ static int fill_autodev(const struct lxc_rootfs *rootfs)
}
fclose(pathfile);
if (safe_mount(hostpath, path, 0, MS_BIND, NULL,
- rootfs->path ? rootfs->mount : NULL) != 0) {
+ rootfs_path ? rootfs_path : NULL) != 0) {
SYSERROR("Failed bind mounting device %s from host into container",
d->name);
return -1;
@@ -1743,7 +1759,7 @@ static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
size_t len = 0;
size_t rootfslen = 0;
- if (!rootfs->path || !lxc_name || !lxc_path)
+ if (!rootfs || !rootfs->path || !lxc_name || !lxc_path)
goto err;
opts = lxc_string_split(mntent->mnt_opts, ',');
@@ -1849,9 +1865,13 @@ 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 ? rootfs->mount : NULL);
+ mntdata, optional, rootfs_path);
free(mntdata);
return ret;
More information about the lxc-devel
mailing list