2020-06-15 07:37:53

by Pavel Begunkov

[permalink] [raw]
Subject: [PATCH 0/2] don't use pid for request cancellation

Cancel requests of an extiting task based on ->task address. As
reported by Eric W. Biederman, using pid for this purpose is not
right.

note: rebased on top of "cancel all" patches

Pavel Begunkov (2):
io_uring: lazy get task
io_uring: cancel by ->task not pid

fs/io-wq.h | 1 -
fs/io_uring.c | 46 ++++++++++++++++++++++++++++------------------
2 files changed, 28 insertions(+), 19 deletions(-)

--
2.24.0


2020-06-15 15:07:21

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/2] don't use pid for request cancellation

On 6/15/20 1:33 AM, Pavel Begunkov wrote:
> Cancel requests of an extiting task based on ->task address. As
> reported by Eric W. Biederman, using pid for this purpose is not
> right.
>
> note: rebased on top of "cancel all" patches

Looks good, and I had the same thought of not grabbing a ref to the
task for the cancel case where we don't need to dereference it.

--
Jens Axboe

2020-06-15 16:05:18

by Pavel Begunkov

[permalink] [raw]
Subject: Re: [PATCH 0/2] don't use pid for request cancellation

On 15/06/2020 18:04, Jens Axboe wrote:
> On 6/15/20 1:33 AM, Pavel Begunkov wrote:
>> Cancel requests of an extiting task based on ->task address. As
>> reported by Eric W. Biederman, using pid for this purpose is not
>> right.
>>
>> note: rebased on top of "cancel all" patches
>
> Looks good, and I had the same thought of not grabbing a ref to the
> task for the cancel case where we don't need to dereference it.

I'm afraid of ABA problem, but this particular case @current shouldn't
go away until ->close is finished.

I was thinking about not get_task() it at all, but it would _at least_
need a way to add a callback on exit of tasks using io_uring to
cancel everything related there. Similarly to how it's done for
work->files using ->close().

--
Pavel Begunkov