diff mbox series

[5/5] io_uring: add IORING_OP_WAITID support

Message ID 20230711204352.214086-6-axboe@kernel.dk (mailing list archive)
State New
Headers show
Series Add io_uring support for waitid | expand

Commit Message

Jens Axboe July 11, 2023, 8:43 p.m. UTC
This adds support for an async version of waitid(2), in a fully async
version. If an event isn't immediately available, wait for a callback
to trigger a retry.

The format of the sqe is as follows:

sqe->len		The 'which', the idtype being queried/waited for.
sqe->fd			The 'pid' (or id) being waited for.
sqe->file_index		The 'options' being set.
sqe->addr2		A pointer to siginfo_t, if any, being filled in.

buf_index, add3, and waitid_flags are reserved/unused for now.
waitid_flags will be used for options for this request type. One
interesting use case may be to add multi-shot support, so that the
request stays armed and posts a notification every time a monitored
process state change occurs.

Note that this does not support rusage, on Arnd's recommendation.

See the waitid(2) man page for details on the arguments.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 include/linux/io_uring_types.h |   2 +
 include/uapi/linux/io_uring.h  |   2 +
 io_uring/Makefile              |   2 +-
 io_uring/cancel.c              |   5 +
 io_uring/io_uring.c            |   3 +
 io_uring/opdef.c               |   9 ++
 io_uring/waitid.c              | 271 +++++++++++++++++++++++++++++++++
 io_uring/waitid.h              |  15 ++
 8 files changed, 308 insertions(+), 1 deletion(-)
 create mode 100644 io_uring/waitid.c
 create mode 100644 io_uring/waitid.h

Comments

Arnd Bergmann July 11, 2023, 9:11 p.m. UTC | #1
On Tue, Jul 11, 2023, at 22:43, Jens Axboe wrote:
> This adds support for an async version of waitid(2), in a fully async
> version. If an event isn't immediately available, wait for a callback
> to trigger a retry.
>
> The format of the sqe is as follows:
>
> sqe->len		The 'which', the idtype being queried/waited for.
> sqe->fd			The 'pid' (or id) being waited for.
> sqe->file_index		The 'options' being set.
> sqe->addr2		A pointer to siginfo_t, if any, being filled in.
>
> buf_index, add3, and waitid_flags are reserved/unused for now.
> waitid_flags will be used for options for this request type. One
> interesting use case may be to add multi-shot support, so that the
> request stays armed and posts a notification every time a monitored
> process state change occurs.
>
> Note that this does not support rusage, on Arnd's recommendation.
>
> See the waitid(2) man page for details on the arguments.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>

Does this require argument conversion for compat tasks?

Even without the rusage argument, I think the siginfo
remains incompatible with 32-bit tasks, unfortunately.

     Arnd
Jens Axboe July 11, 2023, 9:22 p.m. UTC | #2
On 7/11/23 3:11?PM, Arnd Bergmann wrote:
> On Tue, Jul 11, 2023, at 22:43, Jens Axboe wrote:
>> This adds support for an async version of waitid(2), in a fully async
>> version. If an event isn't immediately available, wait for a callback
>> to trigger a retry.
>>
>> The format of the sqe is as follows:
>>
>> sqe->len		The 'which', the idtype being queried/waited for.
>> sqe->fd			The 'pid' (or id) being waited for.
>> sqe->file_index		The 'options' being set.
>> sqe->addr2		A pointer to siginfo_t, if any, being filled in.
>>
>> buf_index, add3, and waitid_flags are reserved/unused for now.
>> waitid_flags will be used for options for this request type. One
>> interesting use case may be to add multi-shot support, so that the
>> request stays armed and posts a notification every time a monitored
>> process state change occurs.
>>
>> Note that this does not support rusage, on Arnd's recommendation.
>>
>> See the waitid(2) man page for details on the arguments.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> Does this require argument conversion for compat tasks?
> 
> Even without the rusage argument, I think the siginfo
> remains incompatible with 32-bit tasks, unfortunately.

Hmm yes good point, if compat_siginfo and siginfo are different, then it
does need handling for that. Would be a trivial addition, I'll make that
change. Thanks Arnd!
Jens Axboe July 11, 2023, 10:18 p.m. UTC | #3
On 7/11/23 3:22 PM, Jens Axboe wrote:
> On 7/11/23 3:11?PM, Arnd Bergmann wrote:
>> On Tue, Jul 11, 2023, at 22:43, Jens Axboe wrote:
>>> This adds support for an async version of waitid(2), in a fully async
>>> version. If an event isn't immediately available, wait for a callback
>>> to trigger a retry.
>>>
>>> The format of the sqe is as follows:
>>>
>>> sqe->len		The 'which', the idtype being queried/waited for.
>>> sqe->fd			The 'pid' (or id) being waited for.
>>> sqe->file_index		The 'options' being set.
>>> sqe->addr2		A pointer to siginfo_t, if any, being filled in.
>>>
>>> buf_index, add3, and waitid_flags are reserved/unused for now.
>>> waitid_flags will be used for options for this request type. One
>>> interesting use case may be to add multi-shot support, so that the
>>> request stays armed and posts a notification every time a monitored
>>> process state change occurs.
>>>
>>> Note that this does not support rusage, on Arnd's recommendation.
>>>
>>> See the waitid(2) man page for details on the arguments.
>>>
>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> Does this require argument conversion for compat tasks?
>>
>> Even without the rusage argument, I think the siginfo
>> remains incompatible with 32-bit tasks, unfortunately.
> 
> Hmm yes good point, if compat_siginfo and siginfo are different, then it
> does need handling for that. Would be a trivial addition, I'll make that
> change. Thanks Arnd!

Should be fixed in the current version:

https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
Christian Brauner July 14, 2023, 3:47 p.m. UTC | #4
On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
> On 7/11/23 3:22 PM, Jens Axboe wrote:
> > On 7/11/23 3:11?PM, Arnd Bergmann wrote:
> >> On Tue, Jul 11, 2023, at 22:43, Jens Axboe wrote:
> >>> This adds support for an async version of waitid(2), in a fully async
> >>> version. If an event isn't immediately available, wait for a callback
> >>> to trigger a retry.
> >>>
> >>> The format of the sqe is as follows:
> >>>
> >>> sqe->len		The 'which', the idtype being queried/waited for.
> >>> sqe->fd			The 'pid' (or id) being waited for.
> >>> sqe->file_index		The 'options' being set.
> >>> sqe->addr2		A pointer to siginfo_t, if any, being filled in.
> >>>
> >>> buf_index, add3, and waitid_flags are reserved/unused for now.
> >>> waitid_flags will be used for options for this request type. One
> >>> interesting use case may be to add multi-shot support, so that the
> >>> request stays armed and posts a notification every time a monitored
> >>> process state change occurs.
> >>>
> >>> Note that this does not support rusage, on Arnd's recommendation.
> >>>
> >>> See the waitid(2) man page for details on the arguments.
> >>>
> >>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> >>
> >> Does this require argument conversion for compat tasks?
> >>
> >> Even without the rusage argument, I think the siginfo
> >> remains incompatible with 32-bit tasks, unfortunately.
> > 
> > Hmm yes good point, if compat_siginfo and siginfo are different, then it
> > does need handling for that. Would be a trivial addition, I'll make that
> > change. Thanks Arnd!
> 
> Should be fixed in the current version:
> 
> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc

In kernel/signal.c in pidfd_send_signal() we have
copy_siginfo_from_user_any() it seems that a similar version
copy_siginfo_to_user_any() might be something to consider. We do have
copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
context why this wouldn't work here.
Arnd Bergmann July 14, 2023, 6:33 p.m. UTC | #5
On Fri, Jul 14, 2023, at 17:47, Christian Brauner wrote:
> On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
>> On 7/11/23 3:22 PM, Jens Axboe wrote:
>> > On 7/11/23 3:11?PM, Arnd Bergmann wrote:

>> >> Does this require argument conversion for compat tasks?
>> >>
>> >> Even without the rusage argument, I think the siginfo
>> >> remains incompatible with 32-bit tasks, unfortunately.
>> > 
>> > Hmm yes good point, if compat_siginfo and siginfo are different, then it
>> > does need handling for that. Would be a trivial addition, I'll make that
>> > change. Thanks Arnd!
>> 
>> Should be fixed in the current version:
>> 
>> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
>
> In kernel/signal.c in pidfd_send_signal() we have
> copy_siginfo_from_user_any() it seems that a similar version
> copy_siginfo_to_user_any() might be something to consider. We do have
> copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
> context why this wouldn't work here.

We could add a copy_siginfo_to_user_any(), but I think open-coding
it is easier here, since the in_compat_syscall() check does not
work inside of the io_uring kernel thread, it has to be
"if (req->ctx->compat)" in order to match the wordsize of the task
that started the request.

Using copy_siginfo_to_user32() and copy_siginfo_to_user() is
probably a good idea though, it's often faster and less
error-prone than writing each member separately.

      Arnd
Jens Axboe July 14, 2023, 8:14 p.m. UTC | #6
On 7/14/23 12:33?PM, Arnd Bergmann wrote:
> On Fri, Jul 14, 2023, at 17:47, Christian Brauner wrote:
>> On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
>>> On 7/11/23 3:22?PM, Jens Axboe wrote:
>>>> On 7/11/23 3:11?PM, Arnd Bergmann wrote:
> 
>>>>> Does this require argument conversion for compat tasks?
>>>>>
>>>>> Even without the rusage argument, I think the siginfo
>>>>> remains incompatible with 32-bit tasks, unfortunately.
>>>>
>>>> Hmm yes good point, if compat_siginfo and siginfo are different, then it
>>>> does need handling for that. Would be a trivial addition, I'll make that
>>>> change. Thanks Arnd!
>>>
>>> Should be fixed in the current version:
>>>
>>> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
>>
>> In kernel/signal.c in pidfd_send_signal() we have
>> copy_siginfo_from_user_any() it seems that a similar version
>> copy_siginfo_to_user_any() might be something to consider. We do have
>> copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
>> context why this wouldn't work here.
> 
> We could add a copy_siginfo_to_user_any(), but I think open-coding
> it is easier here, since the in_compat_syscall() check does not
> work inside of the io_uring kernel thread, it has to be
> "if (req->ctx->compat)" in order to match the wordsize of the task
> that started the request.

Yeah, unifying this stuff did cross my mind when adding another one.
Which I think could still be done, you'd just need to pass in a 'compat'
parameter similar to how it's done for iovec importing.

But if it's ok with everybody I'd rather do that as a cleanup post this.

> Using copy_siginfo_to_user32() and copy_siginfo_to_user() is
> probably a good idea though, it's often faster and less
> error-prone than writing each member separately.

I was just pattern matching on the other use cases. I'll take a look at
the siginfo copy helpers, thanks!
Arnd Bergmann July 15, 2023, 7:12 a.m. UTC | #7
On Fri, Jul 14, 2023, at 22:14, Jens Axboe wrote:
> On 7/14/23 12:33?PM, Arnd Bergmann wrote:
>> On Fri, Jul 14, 2023, at 17:47, Christian Brauner wrote:
>>> On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
>>>>>> Does this require argument conversion for compat tasks?
>>>>>>
>>>>>> Even without the rusage argument, I think the siginfo
>>>>>> remains incompatible with 32-bit tasks, unfortunately.
>>>>>
>>>>> Hmm yes good point, if compat_siginfo and siginfo are different, then it
>>>>> does need handling for that. Would be a trivial addition, I'll make that
>>>>> change. Thanks Arnd!
>>>>
>>>> Should be fixed in the current version:
>>>>
>>>> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
>>>
>>> In kernel/signal.c in pidfd_send_signal() we have
>>> copy_siginfo_from_user_any() it seems that a similar version
>>> copy_siginfo_to_user_any() might be something to consider. We do have
>>> copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
>>> context why this wouldn't work here.
>> 
>> We could add a copy_siginfo_to_user_any(), but I think open-coding
>> it is easier here, since the in_compat_syscall() check does not
>> work inside of the io_uring kernel thread, it has to be
>> "if (req->ctx->compat)" in order to match the wordsize of the task
>> that started the request.
>
> Yeah, unifying this stuff did cross my mind when adding another one.
> Which I think could still be done, you'd just need to pass in a 'compat'
> parameter similar to how it's done for iovec importing.
>
> But if it's ok with everybody I'd rather do that as a cleanup post this.

Sure, keeping that separate seem best.

Looking at what copy_siginfo_from_user_any() actually does, I don't
even think it's worth adapting copy_siginfo_to_user_any() for io_uring,
since it's already just a trivial wrapper, and adding another
argument would add more complexity overall than it saves.

      Arnd
Jens Axboe July 15, 2023, 2:06 p.m. UTC | #8
On 7/15/23 1:12 AM, Arnd Bergmann wrote:
> On Fri, Jul 14, 2023, at 22:14, Jens Axboe wrote:
>> On 7/14/23 12:33?PM, Arnd Bergmann wrote:
>>> On Fri, Jul 14, 2023, at 17:47, Christian Brauner wrote:
>>>> On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
>>>>>>> Does this require argument conversion for compat tasks?
>>>>>>>
>>>>>>> Even without the rusage argument, I think the siginfo
>>>>>>> remains incompatible with 32-bit tasks, unfortunately.
>>>>>>
>>>>>> Hmm yes good point, if compat_siginfo and siginfo are different, then it
>>>>>> does need handling for that. Would be a trivial addition, I'll make that
>>>>>> change. Thanks Arnd!
>>>>>
>>>>> Should be fixed in the current version:
>>>>>
>>>>> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
>>>>
>>>> In kernel/signal.c in pidfd_send_signal() we have
>>>> copy_siginfo_from_user_any() it seems that a similar version
>>>> copy_siginfo_to_user_any() might be something to consider. We do have
>>>> copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
>>>> context why this wouldn't work here.
>>>
>>> We could add a copy_siginfo_to_user_any(), but I think open-coding
>>> it is easier here, since the in_compat_syscall() check does not
>>> work inside of the io_uring kernel thread, it has to be
>>> "if (req->ctx->compat)" in order to match the wordsize of the task
>>> that started the request.
>>
>> Yeah, unifying this stuff did cross my mind when adding another one.
>> Which I think could still be done, you'd just need to pass in a 'compat'
>> parameter similar to how it's done for iovec importing.
>>
>> But if it's ok with everybody I'd rather do that as a cleanup post this.
> 
> Sure, keeping that separate seem best.
> 
> Looking at what copy_siginfo_from_user_any() actually does, I don't
> even think it's worth adapting copy_siginfo_to_user_any() for io_uring,
> since it's already just a trivial wrapper, and adding another
> argument would add more complexity overall than it saves.

Yeah, took a look too this morning, and not sure there's much to reduce
here that would make it cleaner. I'm going to send out a v2 with this
unchanged, holler if people disagree.
Jens Axboe July 15, 2023, 2:34 p.m. UTC | #9
On 7/15/23 8:06?AM, Jens Axboe wrote:
> On 7/15/23 1:12?AM, Arnd Bergmann wrote:
>> On Fri, Jul 14, 2023, at 22:14, Jens Axboe wrote:
>>> On 7/14/23 12:33?PM, Arnd Bergmann wrote:
>>>> On Fri, Jul 14, 2023, at 17:47, Christian Brauner wrote:
>>>>> On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
>>>>>>>> Does this require argument conversion for compat tasks?
>>>>>>>>
>>>>>>>> Even without the rusage argument, I think the siginfo
>>>>>>>> remains incompatible with 32-bit tasks, unfortunately.
>>>>>>>
>>>>>>> Hmm yes good point, if compat_siginfo and siginfo are different, then it
>>>>>>> does need handling for that. Would be a trivial addition, I'll make that
>>>>>>> change. Thanks Arnd!
>>>>>>
>>>>>> Should be fixed in the current version:
>>>>>>
>>>>>> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
>>>>>
>>>>> In kernel/signal.c in pidfd_send_signal() we have
>>>>> copy_siginfo_from_user_any() it seems that a similar version
>>>>> copy_siginfo_to_user_any() might be something to consider. We do have
>>>>> copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
>>>>> context why this wouldn't work here.
>>>>
>>>> We could add a copy_siginfo_to_user_any(), but I think open-coding
>>>> it is easier here, since the in_compat_syscall() check does not
>>>> work inside of the io_uring kernel thread, it has to be
>>>> "if (req->ctx->compat)" in order to match the wordsize of the task
>>>> that started the request.
>>>
>>> Yeah, unifying this stuff did cross my mind when adding another one.
>>> Which I think could still be done, you'd just need to pass in a 'compat'
>>> parameter similar to how it's done for iovec importing.
>>>
>>> But if it's ok with everybody I'd rather do that as a cleanup post this.
>>
>> Sure, keeping that separate seem best.
>>
>> Looking at what copy_siginfo_from_user_any() actually does, I don't
>> even think it's worth adapting copy_siginfo_to_user_any() for io_uring,
>> since it's already just a trivial wrapper, and adding another
>> argument would add more complexity overall than it saves.
> 
> Yeah, took a look too this morning, and not sure there's much to reduce
> here that would make it cleaner. I'm going to send out a v2 with this
> unchanged, holler if people disagree.

Looking over changes, none have been made so far. So I guess a v2 can
wait a bit. The branch was rebased to add Christian's acked-bys for some
of the patches, and since a branch it was based on (io_uring-futex) got
rebased to accommodate PeterZ's changes.
Jens Axboe July 15, 2023, 8:23 p.m. UTC | #10
On 7/15/23 8:06?AM, Jens Axboe wrote:
> On 7/15/23 1:12?AM, Arnd Bergmann wrote:
>> On Fri, Jul 14, 2023, at 22:14, Jens Axboe wrote:
>>> On 7/14/23 12:33?PM, Arnd Bergmann wrote:
>>>> On Fri, Jul 14, 2023, at 17:47, Christian Brauner wrote:
>>>>> On Tue, Jul 11, 2023 at 04:18:13PM -0600, Jens Axboe wrote:
>>>>>>>> Does this require argument conversion for compat tasks?
>>>>>>>>
>>>>>>>> Even without the rusage argument, I think the siginfo
>>>>>>>> remains incompatible with 32-bit tasks, unfortunately.
>>>>>>>
>>>>>>> Hmm yes good point, if compat_siginfo and siginfo are different, then it
>>>>>>> does need handling for that. Would be a trivial addition, I'll make that
>>>>>>> change. Thanks Arnd!
>>>>>>
>>>>>> Should be fixed in the current version:
>>>>>>
>>>>>> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-waitid&id=08f3dc9b7cedbd20c0f215f25c9a7814c6c601cc
>>>>>
>>>>> In kernel/signal.c in pidfd_send_signal() we have
>>>>> copy_siginfo_from_user_any() it seems that a similar version
>>>>> copy_siginfo_to_user_any() might be something to consider. We do have
>>>>> copy_siginfo_to_user32() and copy_siginfo_to_user(). But I may lack
>>>>> context why this wouldn't work here.
>>>>
>>>> We could add a copy_siginfo_to_user_any(), but I think open-coding
>>>> it is easier here, since the in_compat_syscall() check does not
>>>> work inside of the io_uring kernel thread, it has to be
>>>> "if (req->ctx->compat)" in order to match the wordsize of the task
>>>> that started the request.
>>>
>>> Yeah, unifying this stuff did cross my mind when adding another one.
>>> Which I think could still be done, you'd just need to pass in a 'compat'
>>> parameter similar to how it's done for iovec importing.
>>>
>>> But if it's ok with everybody I'd rather do that as a cleanup post this.
>>
>> Sure, keeping that separate seem best.
>>
>> Looking at what copy_siginfo_from_user_any() actually does, I don't
>> even think it's worth adapting copy_siginfo_to_user_any() for io_uring,
>> since it's already just a trivial wrapper, and adding another
>> argument would add more complexity overall than it saves.
> 
> Yeah, took a look too this morning, and not sure there's much to reduce
> here that would make it cleaner. I'm going to send out a v2 with this
> unchanged, holler if people disagree.

One thing we could do is the below, but honestly not sure it's worth the
hassle?


diff --git a/io_uring/waitid.c b/io_uring/waitid.c
index 14ffa07e161a..6de1041c4784 100644
--- a/io_uring/waitid.c
+++ b/io_uring/waitid.c
@@ -43,6 +43,8 @@ static bool io_waitid_compat_copy_si(struct io_waitid *iw, int signo)
 	bool ret;
 
 	infop = (struct compat_siginfo __user *) iw->infop;
+	if (!infop)
+		return true;
 
 	if (!user_write_access_begin(infop, sizeof(*infop)))
 		return false;
@@ -66,32 +68,13 @@ static bool io_waitid_compat_copy_si(struct io_waitid *iw, int signo)
 static bool io_waitid_copy_si(struct io_kiocb *req, int signo)
 {
 	struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
-	bool ret;
-
-	if (!iw->infop)
-		return true;
 
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		return io_waitid_compat_copy_si(iw, signo);
 #endif
 
-	if (!user_write_access_begin(iw->infop, sizeof(*iw->infop)))
-		return false;
-
-	unsafe_put_user(signo, &iw->infop->si_signo, Efault);
-	unsafe_put_user(0, &iw->infop->si_errno, Efault);
-	unsafe_put_user(iw->info.cause, &iw->infop->si_code, Efault);
-	unsafe_put_user(iw->info.pid, &iw->infop->si_pid, Efault);
-	unsafe_put_user(iw->info.uid, &iw->infop->si_uid, Efault);
-	unsafe_put_user(iw->info.status, &iw->infop->si_status, Efault);
-	ret = true;
-done:
-	user_write_access_end();
-	return ret;
-Efault:
-	ret = false;
-	goto done;
+	return siginfo_put_user(iw->infop, &iw->info, signo);
 }
 
 static int io_waitid_finish(struct io_kiocb *req, int ret)
diff --git a/kernel/exit.c b/kernel/exit.c
index 1c9d1cbadcd0..e3a0b6699a23 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1723,6 +1723,28 @@ static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop,
 	return ret;
 }
 
+bool siginfo_put_user(struct siginfo __user *infop, struct waitid_info *wi,
+		      int signo)
+{
+	if (!infop)
+		return true;
+
+	if (!user_write_access_begin(infop, sizeof(*infop)))
+		return false;
+
+	unsafe_put_user(signo, &infop->si_signo, Efault);
+	unsafe_put_user(0, &infop->si_errno, Efault);
+	unsafe_put_user(wi->cause, &infop->si_code, Efault);
+	unsafe_put_user(wi->pid, &infop->si_pid, Efault);
+	unsafe_put_user(wi->uid, &infop->si_uid, Efault);
+	unsafe_put_user(wi->status, &infop->si_status, Efault);
+	user_write_access_end();
+	return true;
+Efault:
+	user_write_access_end();
+	return false;
+}
+
 SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
 		infop, int, options, struct rusage __user *, ru)
 {
@@ -1737,23 +1759,9 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
 		if (ru && copy_to_user(ru, &r, sizeof(struct rusage)))
 			return -EFAULT;
 	}
-	if (!infop)
-		return err;
-
-	if (!user_write_access_begin(infop, sizeof(*infop)))
+	if (siginfo_put_user(infop, &info, signo))
 		return -EFAULT;
-
-	unsafe_put_user(signo, &infop->si_signo, Efault);
-	unsafe_put_user(0, &infop->si_errno, Efault);
-	unsafe_put_user(info.cause, &infop->si_code, Efault);
-	unsafe_put_user(info.pid, &infop->si_pid, Efault);
-	unsafe_put_user(info.uid, &infop->si_uid, Efault);
-	unsafe_put_user(info.status, &infop->si_status, Efault);
-	user_write_access_end();
 	return err;
-Efault:
-	user_write_access_end();
-	return -EFAULT;
 }
 
 long kernel_wait4(pid_t upid, int __user *stat_addr, int options,
diff --git a/kernel/exit.h b/kernel/exit.h
index f10207ba1341..b7e0e32133fa 100644
--- a/kernel/exit.h
+++ b/kernel/exit.h
@@ -27,4 +27,6 @@ long __do_wait(struct wait_opts *wo);
 int kernel_waitid_prepare(struct wait_opts *wo, int which, pid_t upid,
 			  struct waitid_info *infop, int options,
 			  struct rusage *ru, unsigned int *f_flags);
+bool siginfo_put_user(struct siginfo __user *infop, struct waitid_info *wi,
+		      int signo);
 #endif
diff mbox series

Patch

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index a7f03d8d879f..598553877fc2 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -276,6 +276,8 @@  struct io_ring_ctx {
 	struct hlist_head	futex_list;
 	struct io_alloc_cache	futex_cache;
 
+	struct hlist_head	waitid_list;
+
 	const struct cred	*sq_creds;	/* cred used for __io_sq_thread() */
 	struct io_sq_data	*sq_data;	/* if using sq thread polling */
 
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 420f38675769..8fca2cffc343 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -66,6 +66,7 @@  struct io_uring_sqe {
 		__u32		msg_ring_flags;
 		__u32		uring_cmd_flags;
 		__u32		futex_flags;
+		__u32		waitid_flags;
 	};
 	__u64	user_data;	/* data to be passed back at completion time */
 	/* pack this to avoid bogus arm OABI complaints */
@@ -239,6 +240,7 @@  enum io_uring_op {
 	IORING_OP_FUTEX_WAIT,
 	IORING_OP_FUTEX_WAKE,
 	IORING_OP_FUTEX_WAITV,
+	IORING_OP_WAITID,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
diff --git a/io_uring/Makefile b/io_uring/Makefile
index 2e4779bc550c..e5be47e4fc3b 100644
--- a/io_uring/Makefile
+++ b/io_uring/Makefile
@@ -8,6 +8,6 @@  obj-$(CONFIG_IO_URING)		+= io_uring.o xattr.o nop.o fs.o splice.o \
 					statx.o net.o msg_ring.o timeout.o \
 					sqpoll.o fdinfo.o tctx.o poll.o \
 					cancel.o kbuf.o rsrc.o rw.o opdef.o \
-					notif.o
+					notif.o waitid.o
 obj-$(CONFIG_IO_WQ)		+= io-wq.o
 obj-$(CONFIG_FUTEX)		+= futex.o
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index 3dba8ccb1cd8..a01f3f41012b 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -16,6 +16,7 @@ 
 #include "poll.h"
 #include "timeout.h"
 #include "futex.h"
+#include "waitid.h"
 #include "cancel.h"
 
 struct io_cancel {
@@ -124,6 +125,10 @@  int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
 	if (ret != -ENOENT)
 		return ret;
 
+	ret = io_waitid_cancel(ctx, cd, issue_flags);
+	if (ret != -ENOENT)
+		return ret;
+
 	spin_lock(&ctx->completion_lock);
 	if (!(cd->flags & IORING_ASYNC_CANCEL_FD))
 		ret = io_timeout_cancel(ctx, cd);
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 67ff148bc394..6d99d51b84e6 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -93,6 +93,7 @@ 
 #include "net.h"
 #include "notif.h"
 #include "futex.h"
+#include "waitid.h"
 
 #include "timeout.h"
 #include "poll.h"
@@ -336,6 +337,7 @@  static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
 	ctx->submit_state.free_list.next = NULL;
 	INIT_WQ_LIST(&ctx->locked_free_list);
 	INIT_HLIST_HEAD(&ctx->futex_list);
+	INIT_HLIST_HEAD(&ctx->waitid_list);
 	INIT_DELAYED_WORK(&ctx->fallback_work, io_fallback_req_func);
 	INIT_WQ_LIST(&ctx->submit_state.compl_reqs);
 	return ctx;
@@ -3259,6 +3261,7 @@  static __cold bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
 	mutex_lock(&ctx->uring_lock);
 	ret |= io_poll_remove_all(ctx, task, cancel_all);
 	ret |= io_futex_remove_all(ctx, task, cancel_all);
+	ret |= io_waitid_remove_all(ctx, task, cancel_all);
 	mutex_unlock(&ctx->uring_lock);
 	ret |= io_kill_timeouts(ctx, task, cancel_all);
 	if (task)
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 2034acfe10d0..2fbdf6a6c24a 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -34,6 +34,7 @@ 
 #include "cancel.h"
 #include "rw.h"
 #include "futex.h"
+#include "waitid.h"
 
 static int io_no_issue(struct io_kiocb *req, unsigned int issue_flags)
 {
@@ -453,6 +454,10 @@  const struct io_issue_def io_issue_defs[] = {
 		.prep			= io_eopnotsupp_prep,
 #endif
 	},
+	[IORING_OP_WAITID] = {
+		.prep			= io_waitid_prep,
+		.issue			= io_waitid,
+	},
 };
 
 const struct io_cold_def io_cold_defs[] = {
@@ -681,6 +686,10 @@  const struct io_cold_def io_cold_defs[] = {
 	[IORING_OP_FUTEX_WAITV] = {
 		.name			= "FUTEX_WAITV",
 	},
+	[IORING_OP_WAITID] = {
+		.name			= "WAITID",
+		.async_size		= sizeof(struct io_waitid_async),
+	},
 };
 
 const char *io_uring_get_opcode(u8 opcode)
diff --git a/io_uring/waitid.c b/io_uring/waitid.c
new file mode 100644
index 000000000000..8d6ac22113dd
--- /dev/null
+++ b/io_uring/waitid.c
@@ -0,0 +1,271 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for async notification of waitid
+ */
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/io_uring.h>
+
+#include <uapi/linux/io_uring.h>
+
+#include "io_uring.h"
+#include "cancel.h"
+#include "waitid.h"
+#include "../kernel/exit.h"
+
+struct io_waitid {
+	struct file *file;
+	int which;
+	pid_t upid;
+	int options;
+	struct wait_queue_head *head;
+	struct siginfo __user *infop;
+	struct waitid_info info;
+};
+
+static void io_waitid_free(struct io_kiocb *req)
+{
+	struct io_waitid_async *iwa = req->async_data;
+
+	put_pid(iwa->wo.wo_pid);
+	kfree(req->async_data);
+	req->async_data = NULL;
+	req->flags &= ~REQ_F_ASYNC_DATA;
+}
+
+static int io_waitid_finish(struct io_kiocb *req, int ret)
+{
+	struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
+	int signo = 0;
+
+	if (ret > 0) {
+		signo = SIGCHLD;
+		ret = 0;
+	}
+	if (!iw->infop)
+		goto done;
+
+	if (!user_write_access_begin(iw->infop, sizeof(*iw->infop))) {
+		ret = -EFAULT;
+		goto done;
+	}
+
+	unsafe_put_user(signo, &iw->infop->si_signo, Efault);
+	unsafe_put_user(0, &iw->infop->si_errno, Efault);
+	unsafe_put_user(iw->info.cause, &iw->infop->si_code, Efault);
+	unsafe_put_user(iw->info.pid, &iw->infop->si_pid, Efault);
+	unsafe_put_user(iw->info.uid, &iw->infop->si_uid, Efault);
+	unsafe_put_user(iw->info.status, &iw->infop->si_status, Efault);
+done:
+	user_write_access_end();
+	io_waitid_free(req);
+	return ret;
+Efault:
+	ret = -EFAULT;
+	goto done;
+}
+
+static void io_waitid_complete(struct io_kiocb *req, int ret)
+{
+	struct io_tw_state ts = { .locked = true };
+
+	lockdep_assert_held(&req->ctx->uring_lock);
+
+	/*
+	 * Did cancel find it meanwhile?
+	 */
+	if (hlist_unhashed(&req->hash_node))
+		return;
+
+	hlist_del_init(&req->hash_node);
+
+	ret = io_waitid_finish(req, ret);
+	if (ret < 0)
+		req_set_fail(req);
+	io_req_set_res(req, ret, 0);
+	io_req_task_complete(req, &ts);
+}
+
+static bool __io_waitid_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+	struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
+	struct wait_queue_head *head;
+
+	head = READ_ONCE(iw->head);
+	if (head) {
+		struct io_waitid_async *iwa = req->async_data;
+
+		spin_lock_irq(&head->lock);
+		list_del_init(&iwa->wo.child_wait.entry);
+		iw->head = NULL;
+		spin_unlock_irq(&head->lock);
+		io_waitid_complete(req, -ECANCELED);
+		return true;
+	}
+
+	return false;
+}
+
+int io_waitid_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+		     unsigned int issue_flags)
+{
+	struct hlist_node *tmp;
+	struct io_kiocb *req;
+	int nr = 0;
+
+	if (cd->flags & (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED))
+		return -ENOENT;
+
+	io_ring_submit_lock(ctx, issue_flags);
+	hlist_for_each_entry_safe(req, tmp, &ctx->waitid_list, hash_node) {
+		if (req->cqe.user_data != cd->data &&
+		    !(cd->flags & IORING_ASYNC_CANCEL_ANY))
+			continue;
+		if (__io_waitid_cancel(ctx, req))
+			nr++;
+		if (!(cd->flags & IORING_ASYNC_CANCEL_ALL))
+			break;
+	}
+	io_ring_submit_unlock(ctx, issue_flags);
+
+	if (nr)
+		return nr;
+
+	return -ENOENT;
+}
+
+bool io_waitid_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+			  bool cancel_all)
+{
+	struct hlist_node *tmp;
+	struct io_kiocb *req;
+	bool found = false;
+
+	lockdep_assert_held(&ctx->uring_lock);
+
+	hlist_for_each_entry_safe(req, tmp, &ctx->waitid_list, hash_node) {
+		if (!io_match_task_safe(req, task, cancel_all))
+			continue;
+		__io_waitid_cancel(ctx, req);
+		found = true;
+	}
+
+	return found;
+}
+
+static void io_waitid_cb(struct io_kiocb *req, struct io_tw_state *ts)
+{
+	struct io_waitid_async *iwa = req->async_data;
+	struct io_ring_ctx *ctx = req->ctx;
+	int ret;
+
+	/*
+	 * If we get -ERESTARTSYS here, we need to re-arm and check again
+	 * to ensure we get another callback. If the retry works, then we can
+	 * just remove ourselves from the waitqueue again and finish the
+	 * request.
+	 */
+	ret = __do_wait(&iwa->wo);
+	if (unlikely(ret == -ERESTARTSYS)) {
+		struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
+
+		io_tw_lock(ctx, ts);
+		iw->head = &current->signal->wait_chldexit;
+		add_wait_queue(iw->head, &iwa->wo.child_wait);
+		ret = __do_wait(&iwa->wo);
+		if (ret == -ERESTARTSYS)
+			return;
+
+		remove_wait_queue(iw->head, &iwa->wo.child_wait);
+		iw->head = NULL;
+	}
+
+	io_tw_lock(ctx, ts);
+	io_waitid_complete(req, ret);
+}
+
+static int io_waitid_wait(struct wait_queue_entry *wait, unsigned mode,
+			  int sync, void *key)
+{
+	struct wait_opts *wo = container_of(wait, struct wait_opts, child_wait);
+	struct io_waitid_async *iwa = container_of(wo, struct io_waitid_async, wo);
+	struct io_kiocb *req = iwa->req;
+	struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
+	struct task_struct *p = key;
+
+	if (!pid_child_should_wake(wo, p))
+		return 0;
+
+	req->io_task_work.func = io_waitid_cb;
+	io_req_task_work_add(req);
+	iw->head = NULL;
+	list_del_init(&wait->entry);
+	return 1;
+}
+
+int io_waitid_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+	struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
+
+	if (sqe->addr || sqe->buf_index || sqe->addr3 || sqe->waitid_flags)
+		return -EINVAL;
+
+	iw->which = READ_ONCE(sqe->len);
+	iw->options = READ_ONCE(sqe->file_index);
+	iw->upid = READ_ONCE(sqe->fd);
+	iw->infop = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+	iw->head = NULL;
+	return 0;
+}
+
+int io_waitid(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_waitid *iw = io_kiocb_to_cmd(req, struct io_waitid);
+	struct io_ring_ctx *ctx = req->ctx;
+	struct io_waitid_async *iwa;
+	unsigned int f_flags = 0;
+	int ret;
+
+	if (io_alloc_async_data(req))
+		return -ENOMEM;
+
+	iwa = req->async_data;
+	iwa->req = req;
+
+	ret = kernel_waitid_prepare(&iwa->wo, iw->which, iw->upid, &iw->info,
+					iw->options, NULL, &f_flags);
+	if (ret)
+		goto done;
+
+	/*
+	 * Arm our callback and add us to the waitqueue, in case no events
+	 * are available.
+	 */
+	init_waitqueue_func_entry(&iwa->wo.child_wait, io_waitid_wait);
+	iwa->wo.child_wait.private = req->task;
+	iw->head = &current->signal->wait_chldexit;
+	add_wait_queue(iw->head, &iwa->wo.child_wait);
+
+	io_ring_submit_lock(ctx, issue_flags);
+	hlist_add_head(&req->hash_node, &ctx->waitid_list);
+
+	ret = __do_wait(&iwa->wo);
+	if (ret == -ERESTARTSYS) {
+		io_ring_submit_unlock(ctx, issue_flags);
+		return IOU_ISSUE_SKIP_COMPLETE;
+	}
+
+	hlist_del_init(&req->hash_node);
+	remove_wait_queue(iw->head, &iwa->wo.child_wait);
+	iw->head = NULL;
+	ret = io_waitid_finish(req, ret);
+
+	io_ring_submit_unlock(ctx, issue_flags);
+done:
+	if (ret < 0)
+		req_set_fail(req);
+	io_req_set_res(req, ret, 0);
+	return IOU_OK;
+}
diff --git a/io_uring/waitid.h b/io_uring/waitid.h
new file mode 100644
index 000000000000..956a8adafe8c
--- /dev/null
+++ b/io_uring/waitid.h
@@ -0,0 +1,15 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include "../kernel/exit.h"
+
+struct io_waitid_async {
+	struct io_kiocb *req;
+	struct wait_opts wo;
+};
+
+int io_waitid_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_waitid(struct io_kiocb *req, unsigned int issue_flags);
+int io_waitid_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+		     unsigned int issue_flags);
+bool io_waitid_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+			  bool cancel_all);