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

2015-01-27 Thread Christian Seiler
Am 27.01.2015 um 19:02 schrieb Lennart Poettering:
 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.

Looks much nicer, yes.

 Please check if this all works for you now!

Yes, works for me, directory gets created, chown/chmod is applied,
XDG_RUNTIME_DIR is set and loginctl shows the session.

Thanks!

Christian

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2015-01-27 Thread Lennart Poettering
On Fri, 23.01.15 15:26, Christian Seiler (christ...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2015-01-23 Thread Christian Seiler
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.
---
 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

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel