On 1/25/25 01:31, Joanne Koong wrote:
> On Fri, Jan 24, 2025 at 8:47 AM Bernd Schubert <bschubert@xxxxxxx> wrote:
>>
>> The value is read from another task without, while the task that
>> had set the value was holding queue->lock. Better use READ_ONCE
>> to ensure the compiler cannot optimize the read.
>>
>> Fixes: 284985711dc5 ("fuse: Allow to queue fg requests through io-uring")
>> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
>> ---
>> fs/fuse/dev_uring.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
>> index 5c9b5a5fb7f7539149840378e224eb640cf8ef08..1834c1933d2bbab0342257fde4b030f06506c55d 100644
>> --- a/fs/fuse/dev_uring.c
>> +++ b/fs/fuse/dev_uring.c
>> @@ -1202,10 +1202,12 @@ static void fuse_uring_send_in_task(struct io_uring_cmd *cmd,
>> {
>> struct fuse_ring_ent *ent = uring_cmd_to_ring_ent(cmd);
>> struct fuse_ring_queue *queue = ent->queue;
>> + struct fuse_req *req;
>> int err;
>>
>> if (!(issue_flags & IO_URING_F_TASK_DEAD)) {
>> - err = fuse_uring_prepare_send(ent);
>> + req = READ_ONCE(ent->fuse_req);
>> + err = fuse_uring_prepare_send(ent, req);
>
> Hi Bernd, did you mean something like this?:
>
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 5c9b5a5fb7f7..692e97f9870d 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -676,7 +676,7 @@ static int fuse_uring_copy_to_ring(struct
> fuse_ring_ent *ent,
>
> static int fuse_uring_prepare_send(struct fuse_ring_ent *ent)
> {
> - struct fuse_req *req = ent->fuse_req;
> + struct fuse_req *req = READ_ONCE(ent->fuse_req);
> int err;
>
> err = fuse_uring_copy_to_ring(ent, req);
>
> I'm on top of the for-next tree but I'm not seeing where
> fuse_uring_prepare_send() takes in req as an arg.
>
>
Wrong order of patches. Initially it was all in one patch
and I had split it up and was in a hurry - didn't test
compilation of individual patches.
Thanks,
Bernd
