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);
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
Ping on this unapplied patch. Its lack makes it hard to use containers for development of storage systems.
On 28/11/2019 17.59, Miklos Szeredi 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);
linux-stable-mirror@lists.linaro.org
-
Avi Kivity -
Miklos Szeredi -
Miklos Szeredi