Messages in this thread | | | Date | Wed, 23 Nov 2022 12:20:03 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/1] Fix kill(-1,s) returning 0 on 0 kills |
| |
On 11/23, Oleg Nesterov wrote: > > On 11/22, Petr Skocik wrote: > > > > --- a/kernel/signal.c > > +++ b/kernel/signal.c > > @@ -1600,20 +1600,18 @@ static int kill_something_info(int sig, struct kernel_siginfo *info, pid_t pid) > > ret = __kill_pgrp_info(sig, info, > > pid ? find_vpid(-pid) : task_pgrp(current)); > > } else { > > - int retval = 0, count = 0; > > struct task_struct * p; > > > > + ret = -ESRCH; > > for_each_process(p) { > > if (task_pid_vnr(p) > 1 && > > !same_thread_group(p, current)) { > > int err = group_send_sig_info(sig, info, p, > > PIDTYPE_MAX); > > - ++count; > > if (err != -EPERM) > > - retval = err; > > + ret = err; /*either all 0 or all -EINVAL*/ > > The patch looks good to me, and it also simplifies the code. > > But I fail to understand the /*either all 0 or all -EINVAL*/ comment above..
OTOH... I think we do not really care, but there is another problem with or without your patch. Suppose that group_send_sig_info() returns -EAGAIN, then succeeds. So perhaps something like
struct task_struct *p; int esrch = -ESRCH;
ret = 0; for_each_process(p) { if (task_pid_vnr(p) > 1 && !same_thread_group(p, current)) { int err = group_send_sig_info(sig, info, p, PIDTYPE_MAX); if (err == 0) esrch = 0; else if (err != -EPERM) ret = err; } } ret = ret ?: esrch;
if we really want to make this code "100% correct". But again, I am not sure this makes sense.
Oleg.
| |