Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] aio: fix async fsync creds
@ 2019-11-22 10:34 Miklos Szeredi
  2019-11-22 10:40 ` Avi Kivity
  0 siblings, 1 reply; 2+ messages in thread
From: Miklos Szeredi @ 2019-11-22 10:34 UTC (permalink / raw)
  To: Al Viro
  Cc: linux-fsdevel, Christoph Hellwig, Jens Axboe, Giuseppe Scrivano,
	Avi Kivity

From: Miklos Szeredi <mszeredi@redhat.com>

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>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/aio.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- 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,
 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 *
 	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);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] aio: fix async fsync creds
  2019-11-22 10:34 [PATCH] aio: fix async fsync creds Miklos Szeredi
@ 2019-11-22 10:40 ` Avi Kivity
  0 siblings, 0 replies; 2+ messages in thread
From: Avi Kivity @ 2019-11-22 10:40 UTC (permalink / raw)
  To: Miklos Szeredi, Al Viro
  Cc: linux-fsdevel, Christoph Hellwig, Jens Axboe, Giuseppe Scrivano

On 11/22/19 12:34 PM, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@redhat.com>
>
> 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>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>


May I suggest


     Cc: stable@vger.kernel.org  # 4.18+


?


> ---
>   fs/aio.c |    8 ++++++++
>   1 file changed, 8 insertions(+)
>
> --- 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,
>   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 *
>   	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);



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-22 10:34 [PATCH] aio: fix async fsync creds Miklos Szeredi
2019-11-22 10:40 ` Avi Kivity

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git