linux-kernel - [PATCH 2/4] net: freescale/fman-port: remove direct use of __devm_request

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: <20201202161600.23738-2-patrick.havelange@essensium.com>
Date:   Wed,  2 Dec 2020 17:15:58 +0100
From:   Patrick Havelange <patrick.havelange@...ensium.com>
To:     Madalin Bucur <madalin.bucur@....com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Patrick Havelange <patrick.havelange@...ensium.com>
Subject: [PATCH 2/4] net: freescale/fman-port: remove direct use of __devm_request_region

This driver was directly calling __devm_request_region with a specific
resource on the stack as parameter. This is invalid as
__devm_request_region expects the given resource passed as parameter
to live longer than the call itself, as the pointer to the resource
will be stored inside the internal struct used by the devres
management.

In addition to this issue, a related bug has been found by kmemleak
with this trace :
unreferenced object 0xc0000001efc01880 (size 64):
  comm "swapper/0", pid 1, jiffies 4294669078 (age 3620.536s)
  hex dump (first 32 bytes):
    00 00 00 0f fe 4a c0 00 00 00 00 0f fe 4a cf ff  .....J.......J..
    c0 00 00 00 00 ee 9d 98 00 00 00 00 80 00 02 00  ................
  backtrace:
    [<c000000000078874>] .alloc_resource+0xb8/0xe0
    [<c000000000079b50>] .__request_region+0x70/0x1c4
    [<c00000000007a010>] .__devm_request_region+0x8c/0x138
    [<c0000000006e0dc8>] .fman_port_probe+0x170/0x420
    [<c0000000005cecb8>] .platform_drv_probe+0x84/0x108
    [<c0000000005cc620>] .driver_probe_device+0x2c4/0x394
    [<c0000000005cc814>] .__driver_attach+0x124/0x128
    [<c0000000005c9ad4>] .bus_for_each_dev+0xb4/0x110
    [<c0000000005cca1c>] .driver_attach+0x34/0x4c
    [<c0000000005ca9b0>] .bus_add_driver+0x264/0x2a4
    [<c0000000005cd9e0>] .driver_register+0x94/0x160
    [<c0000000005cfea4>] .__platform_driver_register+0x60/0x7c
    [<c000000000f86a00>] .fman_port_load+0x28/0x64
    [<c000000000f4106c>] .do_one_initcall+0xd4/0x1a8
    [<c000000000f412fc>] .kernel_init_freeable+0x1bc/0x2a4
    [<c00000000000180c>] .kernel_init+0x24/0x138

Indeed, the new resource (created in __request_region) will be linked
to the given resource living on the stack, which will end its lifetime
after the function calling __devm_request_region has finished.
Meaning the new resource allocated is no longer reachable.

Now that the main fman driver is no longer reserving the region
used by fman-port, this previous hack is no longer needed
and we can use the regular call to devm_request_mem_region instead,
solving those bugs at the same time.

Signed-off-by: Patrick Havelange <patrick.havelange@...ensium.com>
---
 drivers/net/ethernet/freescale/fman/fman_port.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c
index d9baac0dbc7d..354974939d9d 100644
--- a/drivers/net/ethernet/freescale/fman/fman_port.c
+++ b/drivers/net/ethernet/freescale/fman/fman_port.c
@@ -1878,10 +1878,10 @@ static int fman_port_probe(struct platform_device *of_dev)
 
 	of_node_put(port_node);
 
-	dev_res = __devm_request_region(port->dev, &res, res.start,
-					resource_size(&res), "fman-port");
+	dev_res = devm_request_mem_region(port->dev, res.start,
+					  resource_size(&res), "fman-port");
 	if (!dev_res) {
-		dev_err(port->dev, "%s: __devm_request_region() failed\n",
+		dev_err(port->dev, "%s: devm_request_mem_region() failed\n",
 			__func__);
 		err = -EINVAL;
 		goto free_port;
-- 
2.17.1