[PATCH RESEND (again) 0/2] clk: fix possible use after free

[PATCH RESEND (again) 0/2] clk: fix possible use after free

[Nuno Sá]

The first patch is fixing a possible use after free in case a clk
provider is removed before it's consumer (more details on the commit
message.  

The second patch is a straightforward replacement of the open coded 

hlist_del(&clk->clks_node) ->  clk_core_unlink_consumer(clk)

---
Nuno Sá (2):
      clk: fix clk not being unlinked from consumers list
      clk: use clk_core_unlink_consumer() helper

 drivers/clk/clk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
---
base-commit: 5029c56be4bab3bce7db6d9595b0bbd171a5b154
change-id: 20240611-dev-clk-misc-a9e401b25032
--

Thanks!
- Nuno Sá

[PATCH RESEND (again) 1/2] clk: fix clk not being unlinked from consumers list

[Nuno Sá]

When a clk_hw is registered we add a struct clk handle to it's
consumers list. This handle is created in '__clk_register()' per the
'alloc_clk()' call.

As such, we need to remove this handle when unregistering the
clk_hw. This can actually lead to a use after free if a provider gets
removed before a consumer. When removing the consumer, '__clk_put()' is
called and that will do 'hlist_del(&clk->clks_node)' which will touch in
already freed memory.

Fixes: 1df4046a93e0 ("clk: Combine __clk_get() and __clk_create_clk()")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/clk/clk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8cca52be993f..b11beeca7e55 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -4606,6 +4606,8 @@ void clk_unregister(struct clk *clk)
 	if (clk->core->protect_count)
 		pr_warn("%s: unregistering protected clock: %s\n",
 					__func__, clk->core->name);
+
+	clk_core_unlink_consumer(clk);
 	clk_prepare_unlock();
 
 	kref_put(&clk->core->ref, __clk_release);

-- 
2.45.2

[PATCH RESEND (again) 2/2] clk: use clk_core_unlink_consumer() helper

[Nuno Sá]

There is an helper to remove a consumer from the clk provider list.
Hence, let's use it when releasing a consumer.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b11beeca7e55..ed0731a4b773 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -4764,7 +4764,7 @@ void __clk_put(struct clk *clk)
 		clk->exclusive_count = 0;
 	}
 
-	hlist_del(&clk->clks_node);
+	clk_core_unlink_consumer(clk);
 
 	/* If we had any boundaries on that clock, let's drop them. */
 	if (clk->min_rate > 0 || clk->max_rate < ULONG_MAX)

-- 
2.45.2

Re: [PATCH RESEND (again) 2/2] clk: use clk_core_unlink_consumer() helper

[Stephen Boyd]

Quoting Nuno Sá (2024-07-10 01:40:36)
> There is an helper to remove a consumer from the clk provider list.
> Hence, let's use it when releasing a consumer.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---

Applied to clk-next

Re: [PATCH RESEND (again) 2/2] clk: use clk_core_unlink_consumer() helper

[Nuno Sá]

On Tue, 2024-08-06 at 14:50 -0700, Stephen Boyd wrote:
> Quoting Nuno Sá (2024-07-10 01:40:36)
> > There is an helper to remove a consumer from the clk provider list.
> > Hence, let's use it when releasing a consumer.
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> 
> Applied to clk-next

Hi Stephen,

I realized only this one was applied and not the first patch. Could you please tell
me why it's not being accepted or if I should do it somehow differently? Or it just
needs better look at?

- Nuno Sá 

Re: [PATCH RESEND (again) 2/2] clk: use clk_core_unlink_consumer() helper

[Stephen Boyd]

Quoting Nuno Sá (2024-08-11 22:22:54)
> On Tue, 2024-08-06 at 14:50 -0700, Stephen Boyd wrote:
> > Quoting Nuno Sá (2024-07-10 01:40:36)
> > > There is an helper to remove a consumer from the clk provider list.
> > > Hence, let's use it when releasing a consumer.
> > > 
> > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > > ---
> > 
> > Applied to clk-next
> 
> Hi Stephen,
> 
> I realized only this one was applied and not the first patch. Could you please tell
> me why it's not being accepted or if I should do it somehow differently? Or it just
> needs better look at?
> 

I will re-send the first patch in a day or so. I wrote a pile of KUnit
tests for it.