2004-06-16 18:11:53

by Michael Kerrisk

[permalink] [raw]
Subject: Re: [PATCH 2.6.7] kill(2), killpg(2) wrongly fail with EPERM

Gidday Linus,

> On Wed, 16 Jun 2004, Michael Kerrisk wrote:
> >
> > The following patch for 2.6.7 fixes the problem. Please apply.
>
> How about this imho nicer version instead? It results in the main loop
> being just:
>
> success = 0;
> retval = -ESRCH;
> for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid) {
> int err = group_send_sig_info(sig, info, p);
> success |= !err;
> retval = err;
> }
> return success ? 0 : retval;

Yes, it is nicer.

> which seems sensible. If _any_ group-send succeeded, we want to return
> success (ie this is not a EPERM vs everything else issue).
>
> Does this work for you?

Well, in terms of SUSv3/POSIX, I don't think there's a problem,
since the only errors trhat are specified for kill()/killpg()
are EPERM, ESRCH, and EINVAL (invalid signal number). Aside
from the fact that I didn't spot the nice way, I wrote my
patch as I did since I was worried about the possibility of
some other Linux-specific errno values creeping around in the
woodwork. But a little further investigation seems to show that
there aren't other cases to worry about (EAGAIN in send_sig()
doesn't apply for kill()/killpg(). So, your patch is better,
since simpler. I've tested it, and it works as I would expect
for EPERM.

Thanks,

Michael


> -----
> ===== kernel/signal.c 1.120 vs edited =====
> --- 1.120/kernel/signal.c Wed Jun 9 01:46:51 2004
> +++ edited/kernel/signal.c Wed Jun 16 09:09:51 2004
> @@ -1071,23 +1071,19 @@
> struct task_struct *p;
> struct list_head *l;
> struct pid *pid;
> - int retval;
> - int found;
> + int retval, success;
>
> if (pgrp <= 0)
> return -EINVAL;
>
> - found = 0;
> - retval = 0;
> + success = 0;
> + retval = -ESRCH;
> for_each_task_pid(pgrp, PIDTYPE_PGID, p, l, pid) {
> - int err;
> -
> - found = 1;
> - err = group_send_sig_info(sig, info, p);
> - if (!retval)
> - retval = err;
> + int err = group_send_sig_info(sig, info, p);
> + success |= !err;
> + retval = err;
> }
> - return found ? retval : -ESRCH;
> + return success ? 0 : retval;
> }
>
> int
>

--
+++ Jetzt WLAN-Router f?r alle DSL-Einsteiger und Wechsler +++
GMX DSL-Powertarife zudem 3 Monate gratis* http://www.gmx.net/dsl