linux-kernel - Re: [PATCH] PM / clock_ops: Fix crash in clocks management code if !CONFIG_PM

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: <CAMuHMdVY+vTmSBmH6LOfRdDCk6VRk7tcYfr5EWo0zwaKtP49ag@mail.gmail.com>
Date:	Wed, 1 Oct 2014 21:31:32 +0200
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	Geert Uytterhoeven <geert+renesas@...der.be>,
	Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
	Simon Horman <horms@...ge.net.au>,
	Magnus Damm <magnus.damm@...il.com>,
	Grygorii Strashko <grygorii.strashko@...com>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Linux PM list <linux-pm@...r.kernel.org>,
	Linux-sh list <linux-sh@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] PM / clock_ops: Fix crash in clocks management code if !CONFIG_PM_RUNTIME

Hi Rafael,

On Wed, Oct 1, 2014 at 9:47 PM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> On Wednesday, October 01, 2014 08:38:17 PM Geert Uytterhoeven wrote:
>> Unlike the clocks management code for runtime PM, the code used for
>> system suspend does not check the pm_clock_entry.status field.
>> If pm_clk_acquire() failed, ce->status will be PCE_STATUS_ERROR, and
>> ce->clk will be a negative error code (e.g. 0xfffffffe = -2 = -ENOENT).
>>
>> Depending on the clock implementation, suspend or resume may crash with:
>>
>>     Unable to handle kernel NULL pointer dereference at virtual address 00000026
>>
>> (CCF clk_disable() has an IS_ERR_OR_NULL() check, while CCF clk_enable()
>>  only has a NULL check; pre-CCF implementations may behave differently)
>>
>> While just checking for PCE_STATUS_ERROR would be sufficient, it doesn't
>> hurt to use the same state machine as is done for runtime PM, as this
>> makes the two versions more similar, and eligible for a future
>> consolidation.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
>> ---
>> This crash started to happen on armadillo-legacy during s2ram if
>> CONFIG_PM_RUNTIME is not set after applying "[PATCH v2 07/11] ARM:
>> shmobile: r8a7740/armadillo legacy: Add A4MP pm domain support"
>> (http://www.spinics.net/linux/lists/arm-kernel/msg365438.html), as
>> there's no NULL clock for the HDMI device.
>>
>> Most existing code calling pm_clk_suspend()/pm_clk_resume() is protected
>> by a check for CONFIG_PM_RUNTIME (davinci, keystone, omap1,
>> drivers/sh/pm_runtime.c), so it was not affected by this bug.
>>
>> Exceptions are:
>>   - arch/arm/mach-shmobile/pm-r8a7779.c (marzen),
>>   - arch/arm/mach-shmobile/pm-rmobile.c (r8a7740/armadillo and
>>     sh7372/mackerel),
>> but it's difficult to assess from the code whether the bug is really
>> triggered on these platforms.
>>
>> Grygorii Strashko's "[PATCH v1 2/4] ARM: keystone: pm: switch to use
>> generic pm domains" is not affected, as pm_clk_add_clk() is only called
>> for existing clocks.
>>
>> If it crashes on marzen or mackerel, I think this fix needs to be
>> applied to stable, too. I don't have access to marzen or mackerel boards,
>> though.
>>
>> How to test:
>>   - Build a kernel with CONFIG_PM_SLEEP/CONFIG_SUSPEND enabled, but
>>     CONFIG_PM_RUNTIME disabled,
>>   - echo 0 > /sys/module/printk/parameters/console_suspend,
>>   - echo mem > /sys/power/state,
>>   - wake up using e.g. gpio-keys or serial console activity.
>
> Do I think correctly that this would be 3.18 material?

Yes indeed, so Simon can queue up the R-Mobile PM domain bits that will
trigger this on armadillo for 3.19.

If it can be triggered on marzen or mackerel now, I think we need it in
stable, too.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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/