linux-kernel - Re: [PATCH] signals: do_tkill: don't use tasklist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20080307193159.1AFF127010F@magilla.localdomain>
Date:	Fri,  7 Mar 2008 11:31:59 -0800 (PST)
From:	Roland McGrath <roland@...hat.com>
To:	Oleg Nesterov <oleg@...sign.ru>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] signals: do_tkill: don't use tasklist_lock

> Convert do_tkill() to use rcu_read_lock() + lock_task_sighand() to avoid
> taking tasklist lock.

That part looks good.

> Note that we don't return an error if lock_task_sighand() fails, we
> pretend the task dies after receiving the signal. Otherwise, we should
> fight with the nasty races with mt-exec without having any advantage.

To clarify, this is not a change from the existing behavior.
So your change is fine regardless of this issue.

The case you have in mind is that p was the old group_leader
being replaced by another thread that exec'd, right?

It is the most obscure of nits, but I think it can be wrong to drop a
signal in this case.  If it's a fatal signal (especially SIGKILL),
then either the thread group should be killed or the call should
return an error.  

For the exec case, if p->sighand is cleared that means the
release_task(leader) call at the end of de_thread started.  So by
now, the pid has been transferred to the exec'ing thread.  If we just
restart the lookup, it will find the new thread (or not, and we can
return -ESRCH).

I'm inclined to do that, but it certainly should be a second patch
after this one.

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