[PATCH] drivers/firmware: psci_checker: Add missing destroy_timer_on_stack()

[PATCH] drivers/firmware: psci_checker: Add missing destroy_timer_on_stack()

[Lorenzo Pieralisi]

The PSCI checker suspend_test_thread() function (ie executed for the
suspend test) requires an on-stack timer to carry out the test it
executes; it sets it up through the setup_timer_on_stack() API.

setup_timer_on_stack() requires its counterpart destroy_timer_on_stack()
to be called when the timer is disposed of but the PSCI checker code is
currently missing that call, leaving the timer object in an incosistent
state when the PSCI checker stops the thread executing the suspend
test.

Add the missing destroy_timer_on_stack() call to fix the omission.

Fixes: ea8b1c4a6019 ("drivers: psci: PSCI checker module")
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Reported-by: Kees Cook <keescook@chromium.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 drivers/firmware/psci_checker.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/psci_checker.c b/drivers/firmware/psci_checker.c
index 6523ce9..56cf825 100644
--- a/drivers/firmware/psci_checker.c
+++ b/drivers/firmware/psci_checker.c
@@ -340,6 +340,7 @@ static int suspend_test_thread(void *arg)
 	 * later.
 	 */
 	del_timer(&wakeup_timer);
+	destroy_timer_on_stack(&wakeup_timer);
 
 	if (atomic_dec_return_relaxed(&nb_active_threads) == 0)
 		complete(&suspend_threads_done);
-- 
2.4.12

[PATCH] drivers/firmware: psci_checker: Add missing destroy_timer_on_stack()

[Arnd Bergmann]

On Tue, Oct 24, 2017 at 5:02 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> The PSCI checker suspend_test_thread() function (ie executed for the
> suspend test) requires an on-stack timer to carry out the test it
> executes; it sets it up through the setup_timer_on_stack() API.
>
> setup_timer_on_stack() requires its counterpart destroy_timer_on_stack()
> to be called when the timer is disposed of but the PSCI checker code is
> currently missing that call, leaving the timer object in an incosistent
> state when the PSCI checker stops the thread executing the suspend
> test.
>
> Add the missing destroy_timer_on_stack() call to fix the omission.
>
> Fixes: ea8b1c4a6019 ("drivers: psci: PSCI checker module")
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Reported-by: Kees Cook <keescook@chromium.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Mark Rutland <mark.rutland@arm.com>

Hi Lorenzo,

You addressed the patch 'To: arm at kernel.org', but I'm not entirely
sure what to do with it, it would be nice to be a little more explicit whether
you want us to apply the patch directly or just review it, and which trees
you want it to get merged into.

As you are fixing a regression against v4.10, I would assume you want
it merged into v4.14 with a 'cc: stable' tag to have it backported into v4.13,
correct?

      Arnd

[PATCH] drivers/firmware: psci_checker: Add missing destroy_timer_on_stack()

[Lorenzo Pieralisi]

On Mon, Oct 30, 2017 at 10:17:54AM +0100, Arnd Bergmann wrote:
> On Tue, Oct 24, 2017 at 5:02 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > The PSCI checker suspend_test_thread() function (ie executed for the
> > suspend test) requires an on-stack timer to carry out the test it
> > executes; it sets it up through the setup_timer_on_stack() API.
> >
> > setup_timer_on_stack() requires its counterpart destroy_timer_on_stack()
> > to be called when the timer is disposed of but the PSCI checker code is
> > currently missing that call, leaving the timer object in an incosistent
> > state when the PSCI checker stops the thread executing the suspend
> > test.
> >
> > Add the missing destroy_timer_on_stack() call to fix the omission.
> >
> > Fixes: ea8b1c4a6019 ("drivers: psci: PSCI checker module")
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Reported-by: Kees Cook <keescook@chromium.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> 
> Hi Lorenzo,
> 
> You addressed the patch 'To: arm at kernel.org', but I'm not entirely
> sure what to do with it, it would be nice to be a little more explicit whether
> you want us to apply the patch directly or just review it, and which trees
> you want it to get merged into.

Yes, I was about to reply to you - I should have added some comments
to the patch itself, apologies.

> As you are fixing a regression against v4.10, I would assume you want
> it merged into v4.14 with a 'cc: stable' tag to have it backported into v4.13,
> correct?

Yes it is correct - since the PSCI checker went through ARM SoC I expect
fixes to go through ARM SoC too please (but I should have mentioned the
summary you correctly wrote up above myself).

Thanks,
Lorenzo

[PATCH] drivers/firmware: psci_checker: Add missing destroy_timer_on_stack()

[Arnd Bergmann]

On Mon, Oct 30, 2017 at 10:48 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Mon, Oct 30, 2017 at 10:17:54AM +0100, Arnd Bergmann wrote:
>> On Tue, Oct 24, 2017 at 5:02 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>> > The PSCI checker suspend_test_thread() function (ie executed for the
>> > suspend test) requires an on-stack timer to carry out the test it
>> > executes; it sets it up through the setup_timer_on_stack() API.
>> >
>> > setup_timer_on_stack() requires its counterpart destroy_timer_on_stack()
>> > to be called when the timer is disposed of but the PSCI checker code is
>> > currently missing that call, leaving the timer object in an incosistent
>> > state when the PSCI checker stops the thread executing the suspend
>> > test.
>> >
>> > Add the missing destroy_timer_on_stack() call to fix the omission.
>> >
>> > Fixes: ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> > Reported-by: Kees Cook <keescook@chromium.org>
>> > Cc: Kees Cook <keescook@chromium.org>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>>
>> Hi Lorenzo,
>>
>> You addressed the patch 'To: arm at kernel.org', but I'm not entirely
>> sure what to do with it, it would be nice to be a little more explicit whether
>> you want us to apply the patch directly or just review it, and which trees
>> you want it to get merged into.
>
> Yes, I was about to reply to you - I should have added some comments
> to the patch itself, apologies.

Applied to next/drivers now, thanks!

      Arnd