Re: [PATCH 1/1] signal: make group kill signal fatal

On 05/25, Jiri Slaby wrote:

On 05/25/2009 02:07 AM, Oleg Nesterov wrote:
On 05/24, Jiri Slaby wrote:

__fatal_signal_pending() returns now true only for a non-group sent
sigkill, i. e. for example tgkill, send_sig...

No. Please look at complete_signal(). If we queue a fatal signal,
we always add SIGKILL to any thread.

Ah, thanks. But it looks like it doesn't work well in some cases.
Consider this kernel code:

...
static int release(struct inode *ino, struct file *file)
{
unsigned int a;
printk(KERN_DEBUG "fst\n");
for (a = 0; a < 10; a++) {
printk(KERN_DEBUG "%s: SP=%u FSP=%u pend=%.16lx
shpend=%.16lx\n",
__func__,
signal_pending(current),
fatal_signal_pending(current),
current->pending.signal.sig[0],

current->signal->shared_pending.signal.sig[0]);

...

int main(int argc, char **argv)
{
struct pollfd fds = { .events = POLLIN };
int fd;

fd = open("/dev/m", O_RDONLY);
if (fd < 0)
err(1, "open");
fds.fd = fd;
if (poll(&fds, 1, -1) < 0)
err(2, "poll");
close(fd);
return 0;
}

----------------------------------------------
It outputs:
fst
release: SP=1 FSP=0 pend=0000000000000000 shpend=0000000000000100

Because (I guess) ->release() is called by do_exit()->close_files(),
by this time the private SIGKILL is already dequeued.

If you kill this program, poll() never returns to the user-space.

If the poll isn't there, it works well.

Hmm. this is strange. Do you mean that if this program does
sleep(10000) (or something else) instead of poll() above, it
prints pend != 0 ?



Note. There are known issues with fatal_signal_pending() in exit()
path. But in any case, we should not check ->shared_pending.
And even if ->signal != NULL we must not use ->siglock. Because
schedule() checks fatal_signal_pending() under rq->lock, we can
deadlock. Also, the fact that SIGKILL is actually pending is the
current implementation detail, probably this will be changed.

__fatal_signal_pending() could check SIGNAL_GROUP_EXIT, but we
can't do this now, ->signal can be NULL. Actually signal_group_exit()
is more correct.

And. Why do you need fatal_signal_pending() ? It is special,
should be used by things like wait_event_killable().

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages