Re: [PATCH 01/12] aio: fix async fsync creds

From: Miklos Szeredi <miklos@szeredi.hu>
To: Miklos Szeredi <mszeredi@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, Avi Kivity <avi@scylladb.com>,
	Giuseppe Scrivano <gscrivan@redhat.com>,
	stable <stable@vger.kernel.org>
Subject: Re: [PATCH 01/12] aio: fix async fsync creds
Date: Fri, 13 Dec 2019 10:32:11 +0100	[thread overview]
Message-ID: <CAJfpegun6_cX_6udQNrZSPD+Loum8RDTiwh3k6=NgUFbsm=YLw@mail.gmail.com> (raw)
In-Reply-To: <20191128155940.17530-2-mszeredi@redhat.com>

Hi Al,

Could you please review/apply this patch?

Thanks,
Miklos

On Thu, Nov 28, 2019 at 4:59 PM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> Avi Kivity reports that on fuse filesystems running in a user namespace
> asyncronous fsync fails with EOVERFLOW.
>
> The reason is that f_ops->fsync() is called with the creds of the kthread
> performing aio work instead of the creds of the process originally
> submitting IOCB_CMD_FSYNC.
>
> Fuse sends the creds of the caller in the request header and it needs to
> translate the uid and gid into the server's user namespace.  Since the
> kthread is running in init_user_ns, the translation will fail and the
> operation returns an error.
>
> It can be argued that fsync doesn't actually need any creds, but just
> zeroing out those fields in the header (as with requests that currently
> don't take creds) is a backward compatibility risk.
>
> Instead of working around this issue in fuse, solve the core of the problem
> by calling the filesystem with the proper creds.
>
> Reported-by: Avi Kivity <avi@scylladb.com>
> Tested-by: Giuseppe Scrivano <gscrivan@redhat.com>
> Fixes: c9582eb0ff7d ("fuse: Fail all requests with invalid uids or gids")
> Cc: stable@vger.kernel.org  # 4.18+
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/aio.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 0d9a559d488c..37828773e2fe 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -176,6 +176,7 @@ struct fsync_iocb {
>         struct file             *file;
>         struct work_struct      work;
>         bool                    datasync;
> +       struct cred             *creds;
>  };
>
>  struct poll_iocb {
> @@ -1589,8 +1590,11 @@ static int aio_write(struct kiocb *req, const struct iocb *iocb,
>  static void aio_fsync_work(struct work_struct *work)
>  {
>         struct aio_kiocb *iocb = container_of(work, struct aio_kiocb, fsync.work);
> +       const struct cred *old_cred = override_creds(iocb->fsync.creds);
>
>         iocb->ki_res.res = vfs_fsync(iocb->fsync.file, iocb->fsync.datasync);
> +       revert_creds(old_cred);
> +       put_cred(iocb->fsync.creds);
>         iocb_put(iocb);
>  }
>
> @@ -1604,6 +1608,10 @@ static int aio_fsync(struct fsync_iocb *req, const struct iocb *iocb,
>         if (unlikely(!req->file->f_op->fsync))
>                 return -EINVAL;
>
> +       req->creds = prepare_creds();
> +       if (!req->creds)
> +               return -ENOMEM;
> +
>         req->datasync = datasync;
>         INIT_WORK(&req->work, aio_fsync_work);
>         schedule_work(&req->work);
> --
> 2.21.0
>