[libvirt] [PATCH 0/2] {lxc, util}: Fixing issues with NULL argument for

This serie fixes the argument 'type' of syscall mount(). Valgrind is throwing a memory issue when that parameter is NULL. The best option to fix this issue is replace NULL to "none" filesystem. Julio Faracco (2): lxc: moving 'type' argument to avoid issues with mount() syscall. util: moving 'type' argument to avoid issues with mount() syscall. src/lxc/lxc_container.c | 26 +++++++++++++------------- src/util/vircgroup.c | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) -- 2.17.1

This commit fixes a lots of mount calls inside lxc_container.c file. The NULL value into 'type' argument is causing a memory issue. See commit 794b576c2b for more details. The best approach to fix it is moving NULL to "none" filesytem. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_container.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 665b93a0ac..3a1b2d6819 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -720,7 +720,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) VIR_DEBUG("Pivot via %s", root->src->path); /* root->parent must be private, so make / private. */ - if (mount("", "/", NULL, MS_PRIVATE|MS_REC, NULL) < 0) { + if (mount("", "/", "none", MS_PRIVATE|MS_REC, NULL) < 0) { virReportSystemError(errno, "%s", _("Failed to make root private")); goto err; @@ -757,7 +757,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) } /* ... and mount our root onto it */ - if (mount(root->src->path, newroot, NULL, MS_BIND|MS_REC, NULL) < 0) { + if (mount(root->src->path, newroot, "none", MS_BIND|MS_REC, NULL) < 0) { virReportSystemError(errno, _("Failed to bind %s to new root %s"), root->src->path, newroot); @@ -765,7 +765,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root) } if (root->readonly) { - if (mount(root->src->path, newroot, NULL, MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { + if (mount(root->src->path, newroot, "none", MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { virReportSystemError(errno, _("Failed to make new root %s readonly"), root->src->path); @@ -815,9 +815,9 @@ typedef struct { static const virLXCBasicMountInfo lxcBasicMounts[] = { { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false, false }, - { "/proc/sys", "/proc/sys", NULL, MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false }, - { "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", NULL, MS_BIND, false, false, true }, - { "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", NULL, MS_BIND, false, false, true }, + { "/proc/sys", "/proc/sys", "none", MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false }, + { "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", "none", MS_BIND, false, false, true }, + { "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", "none", MS_BIND, false, false, true }, { "sysfs", "/sys", "sysfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false }, { "securityfs", "/sys/kernel/security", "securityfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true, false }, #if WITH_SELINUX @@ -876,7 +876,7 @@ static int lxcContainerSetReadOnly(void) for (i = 0; i < nmounts; i++) { VIR_DEBUG("Bind readonly %s", mounts[i]); - if (mount(mounts[i], mounts[i], NULL, MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { + if (mount(mounts[i], mounts[i], "none", MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) { virReportSystemError(errno, _("Failed to make mount %s readonly"), mounts[i]); @@ -994,7 +994,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled, } if (bindOverReadonly && - mount(mnt_src, mnt->dst, NULL, + mount(mnt_src, mnt->dst, "none", MS_BIND|MS_REMOUNT|mnt_mflags|MS_RDONLY, NULL) < 0) { virReportSystemError(errno, _("Failed to re-mount %s on %s flags=0x%x"), @@ -1069,7 +1069,7 @@ static int lxcContainerMountFSDev(virDomainDefPtr def, VIR_DEBUG("Trying to %s %s to /dev", def->idmap.nuidmap ? "bind" : "move", path); - if (mount(path, "/dev", NULL, flags, NULL) < 0) { + if (mount(path, "/dev", "none", flags, NULL) < 0) { virReportSystemError(errno, _("Failed to mount %s on /dev"), path); @@ -1105,7 +1105,7 @@ static int lxcContainerMountFSDevPTS(virDomainDefPtr def, VIR_DEBUG("Trying to %s %s to /dev/pts", def->idmap.nuidmap ? "bind" : "move", path); - if (mount(path, "/dev/pts", NULL, flags, NULL) < 0) { + if (mount(path, "/dev/pts", "none", flags, NULL) < 0) { virReportSystemError(errno, _("Failed to mount %s on /dev/pts"), path); @@ -1215,7 +1215,7 @@ static int lxcContainerMountFSBind(virDomainFSDefPtr fs, } } - if (mount(src, fs->dst, NULL, MS_BIND, NULL) < 0) { + if (mount(src, fs->dst, "none", MS_BIND, NULL) < 0) { virReportSystemError(errno, _("Failed to bind mount directory %s to %s"), src, fs->dst); @@ -1224,7 +1224,7 @@ static int lxcContainerMountFSBind(virDomainFSDefPtr fs, if (fs->readonly) { VIR_DEBUG("Binding %s readonly", fs->dst); - if (mount(src, fs->dst, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) { + if (mount(src, fs->dst, "none", MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) { virReportSystemError(errno, _("Failed to make directory %s readonly"), fs->dst); @@ -1549,7 +1549,7 @@ static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs, if (fs->readonly) { VIR_DEBUG("Binding %s readonly", fs->dst); - if (mount(fs->dst, fs->dst, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) { + if (mount(fs->dst, fs->dst, "none", MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) < 0) { virReportSystemError(errno, _("Failed to make directory %s readonly"), fs->dst); -- 2.17.1

This commit fixes a mount call inside virgroup.c file. The NULL value into 'type' argument is causing a memory issue. See commit 794b576c2b for more details. The best approach to fix it is moving NULL to "none" filesytem. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0a31947b0d..e810a3d81d 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3962,7 +3962,7 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot, goto cleanup; } - if (mount(src, group->controllers[i].mountPoint, NULL, MS_BIND, + if (mount(src, group->controllers[i].mountPoint, "none", MS_BIND, NULL) < 0) { virReportSystemError(errno, _("Failed to bind cgroup '%s' on '%s'"), -- 2.17.1

On Tue, Jun 26, 2018 at 11:27:55PM -0300, Julio Faracco wrote:
This serie fixes the argument 'type' of syscall mount(). Valgrind is throwing a memory issue when that parameter is NULL. The best option to fix this issue is replace NULL to "none" filesystem.
There's no memory problem here. It is valid to pass NULL - the kernel would rejected it with EINVAL if it was invalid. This is simply a bug in valgrind, so please update the commit messages to make it clear we're simply avoiding valgrind bugs, not fixing memory problems.
Julio Faracco (2): lxc: moving 'type' argument to avoid issues with mount() syscall. util: moving 'type' argument to avoid issues with mount() syscall.
src/lxc/lxc_container.c | 26 +++++++++++++------------- src/util/vircgroup.c | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-)
-- 2.17.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé
-
Julio Faracco