Re: [PATCH v2 16/23] counter: interrupt-cnt: Convert to new counter registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/27/21 11:59 AM, Greg Kroah-Hartman wrote:
On Mon, Dec 27, 2021 at 10:45:19AM +0100, Uwe Kleine-König wrote:
This fixes device lifetime issues where it was possible to free a live
struct device.

Fixes: a55ebd47f21f ("counter: add IRQ or GPIO based counter")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
  drivers/counter/interrupt-cnt.c | 28 ++++++++++++++++------------
  1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
index 4bf706ef46e2..9e99702470c2 100644
--- a/drivers/counter/interrupt-cnt.c
+++ b/drivers/counter/interrupt-cnt.c
@@ -16,7 +16,6 @@
struct interrupt_cnt_priv {
  	atomic_t count;
-	struct counter_device counter;
  	struct gpio_desc *gpio;
  	int irq;
  	bool enabled;
@@ -148,12 +147,14 @@ static const struct counter_ops interrupt_cnt_ops = {
  static int interrupt_cnt_probe(struct platform_device *pdev)
  {
  	struct device *dev = &pdev->dev;
+	struct counter_device *counter;
  	struct interrupt_cnt_priv *priv;
  	int ret;
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
+	counter = devm_counter_alloc(dev, sizeof(*priv));
I just picked one of these patches at random, nothing specific about
this driver...

You can not have a 'struct device' in memory allocated by devm_*()
functions for the obvious reason that now that memory is being
controlled by a reference count that is OUTSIDE of the structure itself.

So while your goal might be good here, this is not the correct solution
at all, sorry.

Before this patch the memory for the struct device was devm_kzalloc'ed. Which as you point out is a bug.

After this patch the memory is reference counted and will be freed when the last reference is dropped, in the release callback of the struct device.

The alloc function is still a devm_ function, but on 'free' it will only drop the reference to the struct device that it holds. This is a very common pattern that is used by basically any driver subsystem in the kernel.

This is the correct solution to the problem.




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux