linux-kernel - Re: [PATCH 07/13] opp: Allow _generic_set_opp_clk

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: <9b2638e6-b842-8737-e5a0-aeeb84927fce@gmail.com>
Date:   Thu, 21 Jan 2021 23:26:43 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-pm@...r.kernel.org,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Rafael Wysocki <rjw@...ysocki.net>,
        Sibi Sankar <sibis@...eaurora.org>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH 07/13] opp: Allow _generic_set_opp_clk_only() to work for
 non-freq devices

21.01.2021 14:17, Viresh Kumar пишет:
> In order to avoid conditional statements at the caller site, this patch
> updates _generic_set_opp_clk_only() to work for devices that don't
> change frequency (like power domains, etc.). Return 0 if the clk pointer
> passed to this routine is not valid.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
...

Hello Viresh,

Thank you very much for yours effort! I gave a quick test to this series
and instantly found one small issue in this patch.

> +	/* We may reach here for devices which don't change frequency */
> +	if (unlikely(!clk))

I replaced dev_pm_opp_set_voltage() with dev_pm_opp_set_opp() in the
Tegra PD driver and got a crash, which happens because the above line
should be:

	if (IS_ERR(clk))

The opp_table->clk is initialized to ERR_PTR(-ENOENT) if device doesn't
have a clock, like a power domain device in my case.

Everything works good after fixing this patch. I'll keep testing and
will be taking a closer look at the rest of the patches over this weekend.

For the record, here is a backtrace of the crash:

Unable to handle kernel NULL pointer dereference at virtual address 00000016
...
(clk_set_rate) from (_set_opp)
(_set_opp) from (dev_pm_opp_set_opp)
(dev_pm_opp_set_opp) from (tegra_genpd_set_performance_state)
(tegra_genpd_set_performance_state) from (_genpd_set_performance_state)
(_genpd_set_performance_state) from (dev_pm_genpd_set_performance_state)
(dev_pm_genpd_set_performance_state) from (_set_required_opp)
(_set_required_opp) from (_set_opp)
(_set_opp) from (dev_pm_opp_set_rate)
(dev_pm_opp_set_rate) from (host1x_runtime_resume)
(host1x_runtime_resume) from (genpd_runtime_resume)
(genpd_runtime_resume) from (__rpm_callback)
(__rpm_callback) from (rpm_callback)
(rpm_callback) from (rpm_resume)
(rpm_resume) from (__pm_runtime_resume)
(__pm_runtime_resume) from (host1x_probe)
(host1x_probe) from (platform_probe)
(platform_probe) from (really_probe)
(really_probe) from (driver_probe_device)
(driver_probe_device) from (device_driver_attach)
(device_driver_attach) from (__driver_attach)
(__driver_attach) from (bus_for_each_dev)
(bus_for_each_dev) from (bus_add_driver)
(bus_add_driver) from (driver_register)
(driver_register) from (__platform_register_drivers)
(__platform_register_drivers) from (host1x_module_init)
(host1x_module_init) from (do_one_initcall)
(do_one_initcall) from (kernel_init_freeable)
(kernel_init_freeable) from (kernel_init)