linux-kernel - Re: [PATCH] opp: fix memory leak in _allocate_opp

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:   Mon, 28 Dec 2020 10:57:33 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     quanyang.wang@...driver.com
Cc:     Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>,
        Stephan Gerhold <stephan@...hold.net>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] opp: fix memory leak in _allocate_opp_table

On 24-12-20, 18:49, quanyang.wang@...driver.com wrote:
> From: Quanyang Wang <quanyang.wang@...driver.com>
> 
> In function _allocate_opp_table, opp_dev is allocated and referenced
> by opp_table via _add_opp_dev. But in the case that the subsequent calls
> return -EPROBE_DEFER, it will jump to err label and opp_table will be
> freed. Then opp_dev becomes an unreferenced object to cause memory leak.
> So let's call _remove_opp_dev to do the cleanup.
> 
> This fixes the following kmemleak report:
> 
> unreferenced object 0xffff000801524a00 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294892465 (age 84.616s)
>   hex dump (first 32 bytes):
>     40 00 56 01 08 00 ff ff 40 00 56 01 08 00 ff ff  @.V.....@.......
>     b8 52 77 7f 08 00 ff ff 00 3c 4c 00 08 00 ff ff  .Rw......<L.....
>   backtrace:
>     [<00000000b1289fb1>] kmemleak_alloc+0x30/0x40
>     [<0000000056da48f0>] kmem_cache_alloc+0x3d4/0x588
>     [<00000000a84b3b0e>] _add_opp_dev+0x2c/0x88
>     [<0000000062a380cd>] _add_opp_table_indexed+0x124/0x268
>     [<000000008b4c8f1f>] dev_pm_opp_of_add_table+0x20/0x1d8
>     [<00000000e5316798>] dev_pm_opp_of_cpumask_add_table+0x48/0xf0
>     [<00000000db0a8ec2>] dt_cpufreq_probe+0x20c/0x448
>     [<0000000030a3a26c>] platform_probe+0x68/0xd8
>     [<00000000c618e78d>] really_probe+0xd0/0x3a0
>     [<00000000642e856f>] driver_probe_device+0x58/0xb8
>     [<00000000f10f5307>] device_driver_attach+0x74/0x80
>     [<0000000004f254b8>] __driver_attach+0x58/0xe0
>     [<0000000009d5d19e>] bus_for_each_dev+0x70/0xc8
>     [<0000000000d22e1c>] driver_attach+0x24/0x30
>     [<0000000001d4e952>] bus_add_driver+0x14c/0x1f0
>     [<0000000089928aaa>] driver_register+0x64/0x120
> 
> Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER")
> Signed-off-by: Quanyang Wang <quanyang.wang@...driver.com>
> ---
>  drivers/opp/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied with an additional patch. Thanks.

Author: Viresh Kumar <viresh.kumar@...aro.org>
Date:   Mon Dec 28 10:51:04 2020 +0530

    opp: Call the missing clk_put() on error
    
    Fix the clock reference counting by calling the missing clk_put() in the
    error path.
    
    Cc: v5.10 <stable@...r.kernel.org> # v5.10
    Fixes: dd461cd9183f ("opp: Allow dev_pm_opp_get_opp_table() to return -EPROBE_DEFER")
    Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
---
 drivers/opp/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index c9e50836b4c2..8c905aabacc0 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1101,7 +1101,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
        ret = dev_pm_opp_of_find_icc_paths(dev, opp_table);
        if (ret) {
                if (ret == -EPROBE_DEFER)
-                       goto remove_opp_dev;
+                       goto put_clk;
 
                dev_warn(dev, "%s: Error finding interconnect paths: %d\n",
                         __func__, ret);
@@ -1113,6 +1113,9 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 
        return opp_table;
 
+put_clk:
+       if (!IS_ERR(opp_table->clk))
+               clk_put(opp_table->clk);
 remove_opp_dev:
        _remove_opp_dev(opp_dev, opp_table);
 err:

-- 
viresh