linux-kernel - Re: [PATCH 6/6] PM / Sleep: Mitigate race between the freezer and request

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: <201203272337.09073.rjw@sisk.pl>
Date:	Tue, 27 Mar 2012 23:37:08 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Saravana Kannan <skannan@...eaurora.org>,
	Kay Sievers <kay.sievers@...y.org>,
	Greg KH <gregkh@...uxfoundation.org>,
	Christian Lamparter <chunkeey@...glemail.com>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	alan@...rguk.ukuu.org.uk,
	Linux PM mailing list <linux-pm@...r.kernel.org>
Subject: Re: [PATCH 6/6] PM / Sleep: Mitigate race between the freezer and request_firmware()

On Tuesday, March 27, 2012, Stephen Boyd wrote:
> On 03/26/12 13:06, Rafael J. Wysocki wrote:
> > On Monday, March 26, 2012, Stephen Boyd wrote:
> >> On 03/25/12 15:04, Rafael J. Wysocki wrote:
> >>> Index: linux/kernel/power/process.c
> >>> ===================================================================
> >>> --- linux.orig/kernel/power/process.c
> >>> +++ linux/kernel/power/process.c
> >>> @@ -135,6 +135,7 @@ int freeze_processes(void)
> >>>  	error = try_to_freeze_tasks(true);
> >>>  	if (!error) {
> >>>  		printk("done.");
> >>> +		__usermodehelper_reset(UMH_DISABLED);
> >>>  		oom_killer_disable();
> >>>  	}
> >>>  	printk("\n");
> >> nitpick: This doesn't seem to be doing a reset, more like a set. Perhaps
> >> this function should be called __usermodehelper_set()?
> > Yes, you are right.  I changed that before, but somehow managed to send an old
> > patch.  The new version follows.
> 
> Looks good. Thanks.
> 
> > ---
> > From: Rafael J. Wysocki <rjw@...k.pl>
> > Subject: PM / Sleep: Mitigate race between the freezer and request_firmware()
> >
> > There is a race condition between the freezer and request_firmware()
> > such that if request_firmware() is run on one CPU and
> > freeze_processes() is run on another CPU and usermodehelper_disable()
> > called by it succeeds to grab umhelper_sem for writing before
> > usermodehelper_read_trylock() called from request_firmware()
> > acquires it for reading, the request_firmware() will fail and
> > trigger a WARN_ON() complaining that it was called at a wrong time.
> > However, in fact, it wasn't called at a wrong time and
> > freeze_processes() simply happened to be executed simultaneously.
> >
> > To avoid this race, at least in some cases, modify
> > usermodehelper_read_trylock() so that it doesn't fail if the
> > freezing of tasks has just started and hasn't been completed yet.
> > Instead, during the freezing of tasks, it will try to freeze the
> > task that has called it so that it can wait until user space is
> > thawed without triggering the scary warning.
> >
> > For this purpose, change usermodehelper_disabled so that it can
> > take three different values, UMH_ENABLED (0), UMH_FREEZING and
> > UMH_DISABLED.  The first one means that usermode helpers are
> > enabled, the last one means "hard disable" (i.e. the system is not
> > ready for usermode helpers to be used) and the second one
> > is reserved for the freezer.  Namely, when freeze_processes() is
> > started, it sets usermodehelper_disabled to UMH_FREEZING which
> > tells usermodehelper_read_trylock() that it shouldn't fail just
> > yet and should call try_to_freeze() if woken up and cannot
> > return immediately.  This way all freezable tasks that happen
> > to call request_firmware() right before freeze_processes() is
> > started and lose the race for umhelper_sem with it will be
> > frozen and will sleep until thaw_processes() unsets
> > usermodehelper_disabled.  [For the non-freezable callers of
> > request_firmware() the race for umhelper_sem against
> > freeze_processes() is unfortunately unavoidable.]
> 
> Reported-by: Stephen Boyd <sboyd@...eaurora.org>

Yeah, sorry.
--
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/