linux-kernel - Re: [PATCH] rtc: rtc-cmos: Fix event handler registration ordering issue

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Oct 2022 19:11:05 +0100
From:   Mel Gorman <mgorman@...hsingularity.net>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Mario Limonciello <mario.limonciello@....com>,
        linux-rtc@...r.kernel.org, Bjorn Helgaas <helgaas@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] rtc: rtc-cmos: Fix event handler registration ordering
 issue

On Wed, Oct 12, 2022 at 08:07:01PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> Because acpi_install_fixed_event_handler() enables the event
> automatically on success, it is incorrect to call it before the
> handler routine passed to it is ready to handle events.
> 
> Unfortunately, the rtc-cmos driver does exactly the incorrect thing
> by calling cmos_wake_setup(), which passes rtc_handler() to
> acpi_install_fixed_event_handler(), before cmos_do_probe(), because
> rtc_handler() uses dev_get_drvdata() to get to the cmos object
> pointer and the driver data pointer is only populated in
> cmos_do_probe().
> 
> This leads to a NULL pointer dereference in rtc_handler() on boot
> if the RTC fixed event happens to be active at the init time.
> 
> To address this issue, change the initialization ordering of the
> driver so that cmos_wake_setup() is always called after a successful
> cmos_do_probe() call.
> 
> While at it, change cmos_pnp_probe() to call cmos_do_probe() after
> the initial if () statement used for computing the IRQ argument to
> be passed to cmos_do_probe() which is cleaner than calling it in
> each branch of that if () (local variable "irq" can be of type int,
> because it is passed to that function as an argument of type int).
> 
> Note that commit 6492fed7d8c9 ("rtc: rtc-cmos: Do not check
> ACPI_FADT_LOW_POWER_S0") caused this issue to affect a larger number
> of systems, because previously it only affected systems with
> ACPI_FADT_LOW_POWER_S0 set, but it is present regardless of that
> commit.
> 
> Fixes: 6492fed7d8c9 ("rtc: rtc-cmos: Do not check ACPI_FADT_LOW_POWER_S0")
> Fixes: a474aaedac99 ("rtc-cmos: move wake setup from ACPI glue into RTC driver")
> Link: https://lore.kernel.org/linux-acpi/20221010141630.zfzi7mk7zvnmclzy@techsingularity.net/
> Reported-by: Mel Gorman <mgorman@...hsingularity.net>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

Works for me so;

Tested-by: Mel Gorman <mgorman@...hsingularity.net>

-- 
Mel Gorman
SUSE Labs