Re: [lxc-devel] [PATCH] mount_proc_if_needed: only safe mount when rootfs is defined

[Serge Hallyn]

Quoting Bogdan Purcareata (bogdan.purcare...@nxp.com):
> The safe_mount function was introduced in order to address CVE-2015-1335,
> one of the vulnerabilities being a mount with a symlink for the
> destination path. In scenarios such as lxc-execute with no rootfs, the
> destination path is the host /proc, which is previously mounted by the
> host, and is unmounted and mounted again in a new set of namespaces,
> therefore eliminating the need to check for it being a symlink.
> 
> Mount the rootfs normally if the rootfs is NULL, keep the safe mount
> only for scenarios where a different rootfs is defined.
> 
> Signed-off-by: Bogdan Purcareata 

Acked-by: Serge E. Hallyn 

> ---
>  src/lxc/conf.c  |  1 +
>  src/lxc/utils.c | 10 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 632dde3..1e30c0c 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -3509,6 +3509,7 @@ int ttys_shift_ids(struct lxc_conf *c)
>   return 0;
>  }
>  
> +/* NOTE: not to be called from inside the container namespace! */
>  int tmp_proc_mount(struct lxc_conf *lxc_conf)
>  {
>   int mounted;
> diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> index ed8c4c4..e31228d 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -1704,6 +1704,8 @@ int safe_mount(const char *src, const char *dest, const 
> char *fstype,
>   *
>   * Returns < 0 on failure, 0 if the correct proc was already mounted
>   * and 1 if a new proc was mounted.
> + *
> + * NOTE: not to be called from inside the container namespace!
>   */
>  int mount_proc_if_needed(const char *rootfs)
>  {
> @@ -1737,8 +1739,14 @@ int mount_proc_if_needed(const char *rootfs)
>   return 0;
>  
>  domount:
> - if (safe_mount("proc", path, "proc", 0, NULL, rootfs) < 0)
> + if (!strcmp(rootfs,"")) /* rootfs is NULL */
> + ret = mount("proc", path, "proc", 0, NULL);
> + else
> + ret = safe_mount("proc", path, "proc", 0, NULL, rootfs);
> +
> + if (ret < 0)
>   return -1;
> +
>   INFO("Mounted /proc in container for security transition");
>   return 1;
>  }
> -- 
> 1.9.1
> 
> ___
> lxc-devel mailing list
> lxc-devel@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

[lxc-devel] [PATCH] mount_proc_if_needed: only safe mount when rootfs is defined

[Bogdan Purcareata]

The safe_mount function was introduced in order to address CVE-2015-1335,
one of the vulnerabilities being a mount with a symlink for the
destination path. In scenarios such as lxc-execute with no rootfs, the
destination path is the host /proc, which is previously mounted by the
host, and is unmounted and mounted again in a new set of namespaces,
therefore eliminating the need to check for it being a symlink.

Mount the rootfs normally if the rootfs is NULL, keep the safe mount
only for scenarios where a different rootfs is defined.

Signed-off-by: Bogdan Purcareata 
---
 src/lxc/conf.c  |  1 +
 src/lxc/utils.c | 10 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 632dde3..1e30c0c 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3509,6 +3509,7 @@ int ttys_shift_ids(struct lxc_conf *c)
return 0;
 }
 
+/* NOTE: not to be called from inside the container namespace! */
 int tmp_proc_mount(struct lxc_conf *lxc_conf)
 {
int mounted;
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index ed8c4c4..e31228d 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1704,6 +1704,8 @@ int safe_mount(const char *src, const char *dest, const 
char *fstype,
  *
  * Returns < 0 on failure, 0 if the correct proc was already mounted
  * and 1 if a new proc was mounted.
+ *
+ * NOTE: not to be called from inside the container namespace!
  */
 int mount_proc_if_needed(const char *rootfs)
 {
@@ -1737,8 +1739,14 @@ int mount_proc_if_needed(const char *rootfs)
return 0;
 
 domount:
-   if (safe_mount("proc", path, "proc", 0, NULL, rootfs) < 0)
+   if (!strcmp(rootfs,"")) /* rootfs is NULL */
+   ret = mount("proc", path, "proc", 0, NULL);
+   else
+   ret = safe_mount("proc", path, "proc", 0, NULL, rootfs);
+
+   if (ret < 0)
return -1;
+
INFO("Mounted /proc in container for security transition");
return 1;
 }
-- 
1.9.1

___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel