In seccomp_notify_addfd(), when SECCOMP_ADDFD_FLAG_SEND is set,
knotif->state is transitioned from SECCOMP_NOTIFY_SENT to
SECCOMP_NOTIFY_REPLIED before dropping filter->notify_lock and waiting
in wait_for_completion_interruptible(&kaddfd.completion).
If wait_for_completion_interruptible() is interrupted by a signal before
the target task processes the kaddfd entry, seccomp_notify_addfd()
removes kaddfd.list from knotif->addfd via list_del(&kaddfd.list), but
leaves knotif->state set to SECCOMP_NOTIFY_REPLIED.
As a consequence:
1. When the target task wakes in seccomp_do_user_notification(), it
observes knotif->state == SECCOMP_NOTIFY_REPLIED with an empty addfd
list and zeroed knotif->val / knotif->error / knotif->flags, causing
the supervised syscall to prematurely complete with return value 0
without installing the file descriptor.
2. Any subsequent retry of SECCOMP_IOCTL_NOTIF_SEND or
SECCOMP_IOCTL_NOTIF_ADDFD by the supervisor fails with -EINPROGRESS
(or -ENOENT after the target exits) because knotif->state is no
longer SECCOMP_NOTIFY_SENT.
Restore knotif->state = SECCOMP_NOTIFY_SENT when removing an unhandled
SECCOMP_ADDFD_FLAG_SEND request from knotif->addfd, and add a
kselftest (user_notification_addfd_send_interrupted) to cover this race.
Fixes: 7cf97b125455 ("seccomp: Introduce addfd ioctl to seccomp user notifier")
Reviewed-by: Bradley Morgan <[email protected]>
Assisted-by: LLM
Signed-off-by: Hui Peng <[email protected]>
---
Changes in v2:
- Added kselftest regression test (user_notification_addfd_send_interrupted)
in tools/testing/selftests/seccomp/seccomp_bpf.c per Kees Cook.
- Collected Reviewed-by tag from Bradley Morgan.
kernel/seccomp.c | 7 +-
tools/testing/selftests/seccomp/seccomp_bpf.c | 98 +++++++++++++++++++
2 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 86cf4460d69e..94c7ba80a8f7 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1808,10 +1808,13 @@ static long seccomp_notify_addfd(struct seccomp_filter
*filter,
* We need to check again if the addfd request has been handled,
* and if not, we will remove it from the queue.
*/
- if (list_empty(&kaddfd.list))
+ if (list_empty(&kaddfd.list)) {
ret = kaddfd.ret;
- else
+ } else {
list_del(&kaddfd.list);
+ if (addfd.flags & SECCOMP_ADDFD_FLAG_SEND)
+ knotif->state = SECCOMP_NOTIFY_SENT;
+ }
out_unlock:
mutex_unlock(&filter->notify_lock);
--
2.47.3
