Re: [PATCH] io_uring/uring_cmd: add missing READ_ONCE() on shared memory read

[Jens Axboe <axboe@kernel.dk>]

On 1/23/25 7:44 AM, Jann Horn wrote:
> On Thu, Jan 23, 2025 at 1:18?AM Jens Axboe <axboe@kernel.dk> wrote:
>> On 1/22/25 12:38 PM, Jens Axboe wrote:
>>>
>>> On Tue, 21 Jan 2025 17:09:59 +0100, Jann Horn wrote:
>>>> cmd->sqe seems to point to shared memory here; so values should only be
>>>> read from it with READ_ONCE(). To ensure that the compiler won't generate
>>>> code that assumes the value in memory will stay constant, add a
>>>> READ_ONCE().
>>>> The callees io_uring_cmd_getsockopt() and io_uring_cmd_setsockopt() already
>>>> do this correctly.
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [1/1] io_uring/uring_cmd: add missing READ_ONCE() on shared memory read
>>>       commit: 0963dba3dc006b454c54fd019bbbdb931e7a7c70
>>
>> I took a closer look and this isn't necessary. Either ->sqe is a full
>> copy at this point. Should probably be renamed as such... If we want to
>> make this clearer, then we should do:
> 
> Are you sure? On mainline (at commit 21266b8df522), I applied the
> attached diff that basically adds some printf debugging and adds this
> in io_uring_cmd_sock():

Yeah you are right, braino on my part. If we don't go async, it's not
copied. The changed fix is still better, but I'll reword the commit
message to be more accurate.

-- 
Jens Axboe