linux-kernel - Re: [RFC][PATCH 1/3] Freezer: Fix vfork problem

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]

Date:	Tue, 27 Feb 2007 00:25:54 +0300
From:	Oleg Nesterov <oleg@...sign.ru>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Srivatsa Vaddagiri <vatsa@...ibm.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...l.org>, Pavel Machek <pavel@....cz>,
	Aneesh Kumar <aneesh.kumar@...il.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Gautham R Shenoy <ego@...ibm.com>
Subject: Re: [RFC][PATCH 1/3] Freezer: Fix vfork problem

On 02/26, Rafael J. Wysocki wrote:
>
> On Monday, 26 February 2007 17:11, Oleg Nesterov wrote:
> > On 02/26, Srivatsa Vaddagiri wrote:
> > >
> > > On Mon, Feb 26, 2007 at 03:00:43PM +0300, Oleg Nesterov wrote:
> > > > In that case we should also modify ____call_usermodehelper(), otherwise we have
> > > > the same "deadlock" if it is frozen. But this is not so easy to do as I thought
> > > > before.
> > > 
> > > Before ____call_usermodehelper can freeze, it should have entered userspace 
> > > right? By that time, its vfork parent should have definitely woken up,
> > > which should avoid the deadlock you point out?
> > 
> > Ah, yes, thanks for correcting me.
> > 
> > We are doing flush_old_exec() a way before entering userspace of course.
> 
> Well, does it mean the patch is acceptable or should I modify it somehow?

Oh, don't ask me, I don't have a time to study these patches currently :)

_Perhaps_ we can do something better than add explicit checks in freezer_...count,
but I can't suggest anything. Btw, we don't need task_lock() to test current->mm,
and I believe we don't need to check PF_BORROWED_MM there.

"[PATCH 2/3] Freezer: Take kernel_execve into consideration" looks a bit incomplete
to me... I agree, this is a good change for now. But, assuming that we can spawn
an "arbitrary" user-space process from kernel space, we may freeze some kernel
thread which is needed for that user-space task to proceed and notice a signal.

I am starting to suspect that call_usermodehelper() needs a special attention
from freezer, but again, I can't suggest anything, at least right now.

"[PATCH 3/3] Freezer: Prevent ___call_usermodehelper from missing freezing requests"
looks unneeded to me, we should imho drop flush_signals() instead. At least, please
don't call do_not_freeze() under sighand->siglock. This looks as if we have a subtle
reason for this lock, but we don't ? Oh, wait ...  ____call_usermodehelper() does
recalc_sigpending() after flush_signals()! This means we can't lost a "fake" signal
from freezer, so we don't need this patch. Agreed?

Apart from "[PATCH 3/3]", I have nothing against these patches, they fix real problems.

Oleg.

-
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/