Re: [PATCH v5 02/10] io_uring: add support for IORING_OP_MKDIRAT

[Pavel Begunkov <asml.silence@gmail.com>]

On 6/24/21 12:11 PM, Dmitry Kadashev wrote:
> On Wed, Jun 23, 2021 at 6:54 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> On 6/23/21 7:41 AM, Dmitry Kadashev wrote:
>>> 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.
> 
> I'm not sure how the compatibility check reads are special. The code is
> either correct or not. If a compatibility check has correctness problems
> then it's pretty much as bad as any other part of the code having such
> problems, no?

If it reads and verifies a values first, e.g. index into some internal
array, and then compiler plays a joke and reloads it, we might be
absolutely screwed expecting 'segfaults', kernel data leakages and all
the fun stuff.

If that's a compatibility check, whether it's loaded earlier or later,
or whatever, it's not a big deal, the userspace can in any case change
the memory at any moment it wishes, even tightly around the moment
we're reading it.


> That said, I'll just go ahead and use the approach that the rest of the
> code (or rather most of it) uses (no READ_ONCE). If it needs fixing then
> the whole bunch can probably be fixed in one go (either a single patch
> or a series).
> 
> Thanks for your help, Pavel!
> 

-- 
Pavel Begunkov