qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, berrange@redhat.com, linfeng23@huawei.com,
	groug@kaod.org, huangy81@chinatelecom.cn, lvivier@redhat.com,
	lizhijian@cn.fujitsu.com, peterx@redhat.com, vgoyal@redhat.com
Cc: leobras@redhat.com, stefanha@redhat.com, quintela@redhat.com
Subject: [PULL 15/20] virtiofsd: Add capability to change/restore umask
Date: Thu,  1 Jul 2021 15:15:40 +0100	[thread overview]
Message-ID: <20210701141545.193571-16-dgilbert@redhat.com> (raw)
In-Reply-To: <20210701141545.193571-1-dgilbert@redhat.com>

From: Vivek Goyal <vgoyal@redhat.com>

When parent directory has default acl and a file is created in that
directory, then umask is ignored and final file permissions are
determined using default acl instead. (man 2 umask).

Currently, fuse applies the umask and sends modified mode in create
request accordingly. fuse server can set FUSE_DONT_MASK and tell
fuse client to not apply umask and fuse server will take care of
it as needed.

With posix acls enabled, requirement will be that we want umask
to determine final file mode if parent directory does not have
default acl.

So if posix acls are enabled, opt in for FUSE_DONT_MASK. virtiofsd
will set umask of the thread doing file creation. And host kernel
should use that umask if parent directory does not have default
acls, otherwise umask does not take affect.

Miklos mentioned that we already call unshare(CLONE_FS) for
every thread. That means umask has now become property of per
thread and it should be ok to manipulate it in file creation path.

This patch only adds capability to change umask and restore it. It
does not enable it yet. Next few patches will add capability to enable it
based on if user enabled posix_acl or not.

This should fix fstest generic/099.

Reported-by: Luis Henriques <lhenriques@suse.de>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210622150852.1507204-6-vgoyal@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 4dec087bd4..65b2c6fd74 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -122,6 +122,7 @@ struct lo_inode {
 struct lo_cred {
     uid_t euid;
     gid_t egid;
+    mode_t umask;
 };
 
 enum {
@@ -172,6 +173,8 @@ struct lo_data {
     /* An O_PATH file descriptor to /proc/self/fd/ */
     int proc_self_fd;
     int user_killpriv_v2, killpriv_v2;
+    /* If set, virtiofsd is responsible for setting umask during creation */
+    bool change_umask;
 };
 
 static const struct fuse_opt lo_opts[] = {
@@ -1134,7 +1137,8 @@ static void lo_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
  * ownership of caller.
  * TODO: What about selinux context?
  */
-static int lo_change_cred(fuse_req_t req, struct lo_cred *old)
+static int lo_change_cred(fuse_req_t req, struct lo_cred *old,
+                          bool change_umask)
 {
     int res;
 
@@ -1154,11 +1158,14 @@ static int lo_change_cred(fuse_req_t req, struct lo_cred *old)
         return errno_save;
     }
 
+    if (change_umask) {
+        old->umask = umask(req->ctx.umask);
+    }
     return 0;
 }
 
 /* Regain Privileges */
-static void lo_restore_cred(struct lo_cred *old)
+static void lo_restore_cred(struct lo_cred *old, bool restore_umask)
 {
     int res;
 
@@ -1173,6 +1180,9 @@ static void lo_restore_cred(struct lo_cred *old)
         fuse_log(FUSE_LOG_ERR, "setegid(%u): %m\n", old->egid);
         exit(1);
     }
+
+    if (restore_umask)
+        umask(old->umask);
 }
 
 static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent,
@@ -1202,7 +1212,7 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent,
         return;
     }
 
-    saverr = lo_change_cred(req, &old);
+    saverr = lo_change_cred(req, &old, lo->change_umask && !S_ISLNK(mode));
     if (saverr) {
         goto out;
     }
@@ -1211,7 +1221,7 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent,
 
     saverr = errno;
 
-    lo_restore_cred(&old);
+    lo_restore_cred(&old, lo->change_umask && !S_ISLNK(mode));
 
     if (res == -1) {
         goto out;
@@ -1917,7 +1927,7 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
         return;
     }
 
-    err = lo_change_cred(req, &old);
+    err = lo_change_cred(req, &old, lo->change_umask);
     if (err) {
         goto out;
     }
@@ -1928,7 +1938,7 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
     fd = openat(parent_inode->fd, name, fi->flags | O_CREAT | O_EXCL, mode);
     err = fd == -1 ? errno : 0;
 
-    lo_restore_cred(&old);
+    lo_restore_cred(&old, lo->change_umask);
 
     /* Ignore the error if file exists and O_EXCL was not given */
     if (err && (err != EEXIST || (fi->flags & O_EXCL))) {
-- 
2.31.1



  parent reply	other threads:[~2021-07-01 14:34 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 14:15 [PULL 00/20] migration queue Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 01/20] tests: migration-test: Still run the rest even if uffd missing Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 02/20] tests: migration-test: Add dirty ring test Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 03/20] migration: fix the memory overwriting risk in add_to_iovec Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 04/20] migration: Move yank outside qemu_start_incoming_migration() Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 05/20] migration: Allow reset of postcopy_recover_triggered when failed Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 06/20] migration: move wait-unplug loop to its own function Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 07/20] migration: failover: continue to wait card unplug on error Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 08/20] virtiofsd: use GDateTime for formatting timestamp for debug messages Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 09/20] docs: describe the security considerations with virtiofsd xattr mapping Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 10/20] virtiofsd: Don't allow file creation with FUSE_OPEN Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 11/20] virtiofsd: Fix fuse setxattr() API change issue Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 12/20] virtiofsd: Fix xattr operations overwriting errno Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 13/20] virtiofsd: Add support for extended setxattr Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 14/20] virtiofsd: Add umask to seccom allow list Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` Dr. David Alan Gilbert (git) [this message]
2021-07-01 14:15 ` [PULL 16/20] virtiofsd: Switch creds, drop FSETID for system.posix_acl_access xattr Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 17/20] virtiofsd: Add an option to enable/disable posix acls Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 18/20] tests/migration: parse the thread-id key of CpuInfoFast Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 19/20] tests/migration: fix "downtime_limit" type when "migrate-set-parameters" Dr. David Alan Gilbert (git)
2021-07-01 14:15 ` [PULL 20/20] migration/rdma: Use error_report to suppress errno message Dr. David Alan Gilbert (git)
2021-07-05  8:57 ` [PULL 00/20] migration queue Peter Maydell
2021-07-05  9:03   ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210701141545.193571-16-dgilbert@redhat.com \
    --to=dgilbert@redhat.com \
    --cc=berrange@redhat.com \
    --cc=groug@kaod.org \
    --cc=huangy81@chinatelecom.cn \
    --cc=leobras@redhat.com \
    --cc=linfeng23@huawei.com \
    --cc=lizhijian@cn.fujitsu.com \
    --cc=lvivier@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).