| Message ID | 20191128155940.17530-2-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | various vfs patches | expand |
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);
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);