On Tue, Jun 22, 2021 at 6:50 PM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
>
> On 6/22/21 12:41 PM, Pavel Begunkov wrote:
> > On 6/3/21 6:18 AM, Dmitry Kadashev wrote:
> >> IORING_OP_MKDIRAT behaves like mkdirat(2) and takes the same flags
> >> and arguments.
> >>
> >> Signed-off-by: Dmitry Kadashev <dkadashev@xxxxxxxxx>
> >> ---
> >> fs/io_uring.c | 55 +++++++++++++++++++++++++++++++++++
> >> include/uapi/linux/io_uring.h | 1 +
> >> 2 files changed, 56 insertions(+)
> >>
> >> diff --git a/fs/io_uring.c b/fs/io_uring.c
> >> index a1ca6badff36..8ab4eb559520 100644
> >> --- a/fs/io_uring.c
> >> +++ b/fs/io_uring.c
> >> @@ -665,6 +665,13 @@ struct io_unlink {
> >> struct filename *filename;
> >> };
> >>
> >> +struct io_mkdir {
> >> + struct file *file;
> >> + int dfd;
> >> + umode_t mode;
> >> + struct filename *filename;
> >> +};
> >> +
> >> struct io_completion {
> >> struct file *file;
> >> struct list_head list;
> >> @@ -809,6 +816,7 @@ struct io_kiocb {
> >> struct io_shutdown shutdown;
> >> struct io_rename rename;
> >> struct io_unlink unlink;
> >> + struct io_mkdir mkdir;
> >> /* use only after cleaning per-op data, see io_clean_op() */
> >> struct io_completion compl;
> >> };
> >> @@ -1021,6 +1029,7 @@ static const struct io_op_def io_op_defs[] = {
> >> },
> >> [IORING_OP_RENAMEAT] = {},
> >> [IORING_OP_UNLINKAT] = {},
> >> + [IORING_OP_MKDIRAT] = {},
> >> };
> >>
> >> static bool io_disarm_next(struct io_kiocb *req);
> >> @@ -3530,6 +3539,44 @@ static int io_unlinkat(struct io_kiocb *req, unsigned int issue_flags)
> >> return 0;
> >> }
> >>
> >> +static int io_mkdirat_prep(struct io_kiocb *req,
> >> + const struct io_uring_sqe *sqe)
> >> +{
> >> + struct io_mkdir *mkd = &req->mkdir;
> >> + const char __user *fname;
> >> +
> >> + if (unlikely(req->flags & REQ_F_FIXED_FILE))
> >> + return -EBADF;
> >> +
> >> + mkd->dfd = READ_ONCE(sqe->fd);
> >> + mkd->mode = READ_ONCE(sqe->len);
> >> +
> >> + fname = u64_to_user_ptr(READ_ONCE(sqe->addr));
> >> + mkd->filename = getname(fname);
> >> + if (IS_ERR(mkd->filename))
> >> + return PTR_ERR(mkd->filename);
> >
> > We have to check unused fields, e.g. buf_index and off,
> > to be able to use them in the future if needed.
> >
> > if (sqe->buf_index || sqe->off)
> > return -EINVAL;
> >
> > Please double check what fields are not used, and
> > same goes for all other opcodes.
This changeset is based on some other ops that were added a while ago
(renameat, unlinkat), which lack the check as well. I guess I'll just go over
all of them and add the checks in a single patch if that's OK.
I'd imagine READ_ONCE is to be used in those checks though, isn't it? Some of
the existing checks like this lack it too btw. I suppose I can fix those in a
separate commit if that makes sense.
>
> + opcode specific flags, e.g.
>
> if (sqe->rw_flags)
> return -EINVAL;
--
Dmitry Kadashev
