[libvirt] [PATCH v2 0/2] {lxc, util}: Fix issues with NULL argument 'type' for mount().
This serie fixes the argument 'type' of syscall mount(). Valgrind is throwing an issue when that parameter 'type' of the syscall is NULL. The best option to fix this issue is replace NULL to "none" filesystem. For more details about the Valgrind bug, see the commit message at 794b576c. 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 valgrind 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 valgrind issue. See commit 794b576c 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 06/27/2018 05:06 PM, Julio Faracco wrote:
This serie fixes the argument 'type' of syscall mount(). Valgrind is throwing an issue when that parameter 'type' of the syscall is NULL. The best option to fix this issue is replace NULL to "none" filesystem. For more details about the Valgrind bug, see the commit message at 794b576c.
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(-)
ACK to the patches. However, I'm not quite sure about safe-for-freeze. I mean, patches will not break anything, but at the same time, it's not that important bug they are fixing, or? What is your opinion in this? Is is okay if I push them after the release? Michal
Hi Michael, Actually, "none" is not used. It is not the proper type there, but since it is not used, we can go ahead. If we check the documentation, we can see the text "The filesystemtype and data arguments are ignored." in options BIND, MOVE and REMOUNT. So, no matter which option we put, the syscall will ignore. -- Julio Cesar Faracco Em qui, 28 de jun de 2018 às 05:02, Michal Prívozník <mprivozn@redhat.com> escreveu:
On 06/27/2018 05:06 PM, Julio Faracco wrote:
This serie fixes the argument 'type' of syscall mount(). Valgrind is throwing an issue when that parameter 'type' of the syscall is NULL. The best option to fix this issue is replace NULL to "none" filesystem. For more details about the Valgrind bug, see the commit message at 794b576c.
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(-)
ACK to the patches. However, I'm not quite sure about safe-for-freeze. I mean, patches will not break anything, but at the same time, it's not that important bug they are fixing, or? What is your opinion in this? Is is okay if I push them after the release?
Michal
participants (2)
-
Julio Faracco -
Michal Prívozník