* Re: [PATCH 2.6.7] kill(2), killpg(2) wrongly fail with EPERM
2004-06-16 16:12 ` [PATCH 2.6.7] kill(2), killpg(2) wrongly fail with EPERM Linus Torvalds
@ 2004-06-16 18:07 ` Michael Kerrisk
0 siblings, 0 replies; 2+ messages in thread
From: Michael Kerrisk @ 2004-06-16 18:07 UTC (permalink / raw)
To: Linus Torvalds; +Cc: akpm, linux-kernel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="us-ascii", Size: 2290 bytes --]
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
^ permalink raw reply [flat|nested] 2+ messages in thread