On 6/23/21 7:41 AM, Dmitry Kadashev wrote: > 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> >>>> --- [...] >>> 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 That's not great... Thanks for letting know > all of them and add the checks in a single patch if that's OK. For newly added opcodes a single patch on top is ok, rename and unlink should be patched separately. > 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. When we really use a field there should be a READ_ONCE(), but I wouldn't care about those we check for compatibility reasons, but that's only my opinion. >> + opcode specific flags, e.g. >> >> if (sqe->rw_flags) >> return -EINVAL; > -- Pavel Begunkov
