[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/4] export/fuse: Let permissions be adjustable
From: |
Kevin Wolf |
Subject: |
Re: [PATCH 3/4] export/fuse: Let permissions be adjustable |
Date: |
Tue, 22 Jun 2021 17:02:47 +0200 |
Am 14.06.2021 um 16:44 hat Max Reitz geschrieben:
> Allow changing the file mode, UID, and GID through SETATTR.
>
> This only really makes sense with allow-other, though (because without
> it, the effective access mode is fixed to be 0600 (u+rw) with qemu's
> user being the file's owner), so changing these stat fields is not
> allowed without allow-other.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/export/fuse.c | 48 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 1d54286d90..742e0af657 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -47,6 +47,10 @@ typedef struct FuseExport {
> bool writable;
> bool growable;
> bool allow_other;
> +
> + mode_t st_mode;
> + uid_t st_uid;
> + gid_t st_gid;
> } FuseExport;
>
> static GHashTable *exports;
> @@ -120,6 +124,13 @@ static int fuse_export_create(BlockExport *blk_exp,
> exp->growable = args->growable;
> exp->allow_other = args->allow_other;
>
> + exp->st_mode = S_IFREG | S_IRUSR;
> + if (exp->writable) {
> + exp->st_mode |= S_IWUSR;
> + }
> + exp->st_uid = getuid();
> + exp->st_gid = getgid();
> +
> ret = setup_fuse_export(exp, args->mountpoint, errp);
> if (ret < 0) {
> goto fail;
> @@ -329,7 +340,6 @@ static void fuse_getattr(fuse_req_t req, fuse_ino_t inode,
> int64_t length, allocated_blocks;
> time_t now = time(NULL);
> FuseExport *exp = fuse_req_userdata(req);
> - mode_t mode;
>
> length = blk_getlength(exp->common.blk);
> if (length < 0) {
> @@ -344,17 +354,12 @@ static void fuse_getattr(fuse_req_t req, fuse_ino_t
> inode,
> allocated_blocks = DIV_ROUND_UP(allocated_blocks, 512);
> }
>
> - mode = S_IFREG | S_IRUSR;
> - if (exp->writable) {
> - mode |= S_IWUSR;
> - }
> -
> statbuf = (struct stat) {
> .st_ino = inode,
> - .st_mode = mode,
> + .st_mode = exp->st_mode,
> .st_nlink = 1,
> - .st_uid = getuid(),
> - .st_gid = getgid(),
> + .st_uid = exp->st_uid,
> + .st_gid = exp->st_gid,
> .st_size = length,
> .st_blksize = blk_bs(exp->common.blk)->bl.request_alignment,
> .st_blocks = allocated_blocks,
> @@ -400,15 +405,23 @@ static int fuse_do_truncate(const FuseExport *exp,
> int64_t size,
> }
>
> /**
> - * Let clients set file attributes. Only resizing is supported.
> + * Let clients set file attributes. With allow_other, only resizing and
> + * changing permissions (st_mode, st_uid, st_gid) is allowed. Without
> + * allow_other, only resizing is supported.
> */
> static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat
> *statbuf,
> int to_set, struct fuse_file_info *fi)
> {
> FuseExport *exp = fuse_req_userdata(req);
> + int supported_attrs;
> int ret;
>
> - if (to_set & ~FUSE_SET_ATTR_SIZE) {
> + supported_attrs = FUSE_SET_ATTR_SIZE;
> + if (exp->allow_other) {
> + supported_attrs |= FUSE_SET_ATTR_MODE | FUSE_SET_ATTR_UID |
> + FUSE_SET_ATTR_GID;
> + }
> + if (to_set & ~supported_attrs) {
> fuse_reply_err(req, ENOTSUP);
> return;
> }
> @@ -426,6 +439,19 @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t
> inode, struct stat *statbuf,
> }
> }
>
> + if (to_set & FUSE_SET_ATTR_MODE) {
> + /* Only allow changing the file mode, not the type */
> + exp->st_mode = (statbuf->st_mode & 07777) | S_IFREG;
> + }
Should we check that the mode actually makes sense? Not sure if making
an image executable has a good use case, and making it writable in the
permissions for a read-only export isn't a good idea either.
> +
> + if (to_set & FUSE_SET_ATTR_UID) {
> + exp->st_uid = statbuf->st_uid;
> + }
> +
> + if (to_set & FUSE_SET_ATTR_GID) {
> + exp->st_gid = statbuf->st_gid;
> + }
> +
> fuse_getattr(req, inode, fi);
> }
Kevin