[systemd-devel] [PATCH 2/2] logind: chown+chmod /run/user/$UID if mount(tmpfs) fails with EPERM

[Lennart Poettering]

On Fri, 23.01.15 15:26, Christian Seiler (christian at iwakd.de) wrote:

> In containers without CAP_SYS_ADMIN, it is not possible to mount tmpfs
> (or any filesystem for that matter) on top of /run/user/$UID.
> Previously, logind just failed in such a situation.
> 
> Now, logind will resort to chown+chmod of the directory instead. This
> allows logind still to work in those environments, although without the
> guarantees it provides (i.e. users not being able to DOS /run or other
> users' /run/user/$UID space) when CAP_SYS_ADMIN is available.

Merged this one too, made some changes first howver. I reworked this
to use our chmod_and_chown() helper, and removed the bit that checks
whether the mount point actually was a mount point after umount2(). I
really prefer if we can just check the return value of the syscall and
decide on that.

Please check if this all works for you now!

Thanks!

> ---
>  src/login/logind-user.c | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/src/login/logind-user.c b/src/login/logind-user.c
> index d7930ad..3f7e3ce 100644
> --- a/src/login/logind-user.c
> +++ b/src/login/logind-user.c
> @@ -335,12 +335,28 @@ static int user_mkdir_runtime_path(User *u) {
>                  }
>  
>                  r = mount("tmpfs", p, "tmpfs", MS_NODEV|MS_NOSUID, t);
> -                if (r < 0) {
> +                if (r < 0 && errno != EPERM) {
>                          /* try to clean up, but ignore errors */
>                          r = -errno;
>                          rmdir(p);
>                          log_error_errno(r, "Failed to mount per-user tmpfs directory %s: %m", p);
>                          goto fail;
> +                } else if (r < 0 && errno == EPERM) {
> +                        /* we probably don't have CAP_SYS_ADMIN and are in a
> +                         * container, so just try to chown()/chmod() the
> +                         * directory. */
> +                        log_debug("Failed to mount per-user tmpfs directory %s, just setting permissions.", p);
> +
> +                        r = chown(p, u->uid, u->gid);
> +                        if (r >= 0)
> +                                r = chmod(p, 0700);
> +
> +                        if (r < 0) {
> +                                r = -errno;
> +                                rmdir(p);
> +                                log_error_errno(r, "Failed to change permissions of per-user tmpfs directory %s: %m", p);
> +                                goto fail;
> +                        }
>                  }
>          }
>  
> @@ -513,8 +529,17 @@ static int user_remove_runtime_path(User *u) {
>          if (r < 0)
>                  log_error_errno(r, "Failed to remove runtime directory %s: %m", u->runtime_path);
>  
> -        if (umount2(u->runtime_path, MNT_DETACH) < 0)
> -                log_error_errno(errno, "Failed to unmount user runtime directory %s: %m", u->runtime_path);
> +        r = umount2(u->runtime_path, MNT_DETACH);
> +        if (r < 0) {
> +                r = -errno;
> +                /* only log an error if the directory was a mount point,
> +                 * otherwise it could just be that we weren't able to
> +                 * mount it because we don't have CAP_SYS_AMDIN. */
> +                if (path_is_mount_point(u->runtime_path, false) > 0)
> +                        log_error_errno(r, "Failed to unmount user runtime directory %s: %m", u->runtime_path);
> +                else
> +                        log_debug_errno(r, "Failed to unmount user runtime directory %s: %m", u->runtime_path);
> +        }
>  
>          r = rm_rf(u->runtime_path, false, true, false);
>          if (r < 0)
> -- 
> 2.1.4
> 
> 


Lennart

-- 
Lennart Poettering, Red Hat