linux-kernel - Re: [PATCH v2] PM/Hibernation: Fix the early termination of test modes

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:	Fri, 18 Nov 2011 23:00:43 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
Cc:	len.brown@...el.com, pavel@....cz, rdunlap@...otime.net,
	tj@...nel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v2] PM/Hibernation: Fix the early termination of test modes

On Friday, November 18, 2011, Srivatsa S. Bhat wrote:
> On 11/18/2011 02:57 PM, Srivatsa S. Bhat wrote:
> > On 11/18/2011 02:50 PM, Rafael J. Wysocki wrote:
> >>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> >>> index b4511b6..257e8e7 100644
> >>> --- a/kernel/power/hibernate.c
> >>> +++ b/kernel/power/hibernate.c
> >>> @@ -55,6 +55,8 @@ enum {
> >>>  
> >>>  static int hibernation_mode = HIBERNATION_SHUTDOWN;
> >>>  
> >>> +static bool freezer_test_done;
> >>> +
> >>>  static const struct platform_hibernation_ops *hibernation_ops;
> >>>  
> >>>  /**
> >>> @@ -347,6 +349,17 @@ int hibernation_snapshot(int platform_mode)
> >>>  	if (error)
> >>>  		goto Close;
> >>>  
> >>> +	if (hibernation_test(TEST_FREEZER) ||
> >>> +		hibernation_testmode(HIBERNATION_TESTPROC)) {
> >>> +
> >>> +		/*
> >>> +		 * Indicate to the caller that we are returning due to a
> >>> +		 * successful freezer test.
> >>> +		 */
> >>> +		freezer_test_done = true;
> >>> +		goto Close;
> >>> +	}
> >>> +
> >>>  	error = dpm_prepare(PMSG_FREEZE);
> >>>  	if (error)
> >>>  		goto Complete_devices;
> >>> @@ -641,15 +654,13 @@ int hibernate(void)
> >>>  	if (error)
> >>>  		goto Finish;
> >>>  
> >>> -	if (hibernation_test(TEST_FREEZER))
> >>> -		goto Thaw;
> >>> -
> >>> -	if (hibernation_testmode(HIBERNATION_TESTPROC))
> >>> -		goto Thaw;
> >>> -
> >>>  	error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
> >>>  	if (error)
> >>>  		goto Thaw;
> >>> +	if (freezer_test_done) {
> >>> +		freezer_test_done = false;
> >>> +		goto Thaw;
> >>> +	}
> >>
> >> What happens if hibernation_snapshot() and freezer_test_done is 'true'?
> >> Won't that leek freezer_test_done to the next hibernation cycle?
> >>
> > 
> > Oh yes, thanks for catching that! I'll fix that and send an updated patch.
> >
> 
> Sorry, I think I hit reply too soon.
> To restate your question, you are asking if freezer_test_done was set in
> hibernation_snapshot() and then there was a genuine error in
> hibernation_snapshot() which it returned, then won't freezer_test_done
> leak to the next cycle.., right?
> 
> If there is a genuine error before hibernation_test(TEST_FREEZER)... is
> done, then we will never set freezer_test_done flag to true.
> 
> And whenever freezer_test_done is set (inside hibernation_snapshot) there
> is no way it will continue further executing normal code. It will jump to
> the Close label and come out, so that here in hibernate(), the first
> check for error will fail (because it returned 0) and the subsequent
> check for freezer_test_done will succeed. So I don't see how it can leak.
> 
> Or, am I missing something?

No, that's fine, I just wasn't sure. :-)

I'm going to apply your patch without the documentation changes.

Thanks,
Rafael
--
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/