Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Stephan Gerhold <stephan.gerhold@kernkonzept.com>
Date:   Wed, 24 May 2023 19:56:21 +0200
Subject: [PATCH] opp: Fix use-after-free in lazy_opp_tables after probe
 deferral
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Message-Id: <20230524-opp-lazy-uaf-v1-1-f5f95cb4b6de@kernkonzept.com>
To:     Viresh Kumar <vireshk@kernel.org>
Cc:     Nishanth Menon <nm@ti.com>, Stephen Boyd <sboyd@kernel.org>,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org,
        Stephan Gerhold <stephan.gerhold@kernkonzept.com>
Precedence: bulk

When dev_pm_opp_of_find_icc_paths() in _allocate_opp_table() returns
-EPROBE_DEFER, the opp_table is freed again, to wait until all the
interconnect paths are available.

However, if the OPP table is using required-opps then it may already
have been added to the global lazy_opp_tables list. The error path
does not remove the opp_table from the list again.

This can cause crashes later when the provider of the required-opps
is added, since we will iterate over OPP tables that have already been
freed. E.g.:

  Unable to handle kernel NULL pointer dereference when read
  CPU: 0 PID: 7 Comm: kworker/0:0 Not tainted 6.4.0-rc3
  PC is at _of_add_opp_table_v2 (include/linux/of.h:949
  drivers/opp/of.c:98 drivers/opp/of.c:344 drivers/opp/of.c:404
  drivers/opp/of.c:1032) -> lazy_link_required_opp_table()

Fix this by removing the opp_table from the list before freeing it.

Cc: stable@vger.kernel.org
Fixes: 7eba0c7641b0 ("opp: Allow lazy-linking of required-opps")
Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
---
This fixes the crash I ran into after adding an OPP table with
both "required-opps" and interconnect paths (opp-peak-kBps).

By the way, the "lazy_opp_tables" does not seem to be protected by any
locks(?) so I could imagine that theoretically there could be a race
condition while adding/removing OPP tables there. This is unrelated
to the crash I saw, though.
---
 drivers/opp/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 85cbc8de407c..6a3a320be7df 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1358,6 +1358,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 	return opp_table;
 
 remove_opp_dev:
+	list_del(&opp_table->lazy);
 	_remove_opp_dev(opp_dev, opp_table);
 err:
 	kfree(opp_table);

---
base-commit: 9e28f7a74581204807f20ae46568939038e327aa
change-id: 20230524-opp-lazy-uaf-60a004b385ec

Best regards,
-- 
Stephan Gerhold
Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth