[PATCH 0/3 1/3] auxdisplay: line-display: add devm_linedisp_register()

kr494167@gmail.com posted 3 patches 1 month, 1 week ago
Only 0 patches received!
drivers/auxdisplay/line-display.c | 32 +++++++++++++++++++++++++++++++
drivers/auxdisplay/line-display.h |  2 ++
2 files changed, 34 insertions(+)
[PATCH 0/3 1/3] auxdisplay: line-display: add devm_linedisp_register()
Posted by kr494167@gmail.com 1 month, 1 week ago
From: Surendra Singh Chouhan <kr494167@gmail.com>

Add devm_linedisp_register() to manage character line display registration
via devres. This simplifies driver cleanup and prevents use-after-free
bugs when unregistering line displays on driver detach.

Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/auxdisplay/line-display.c | 32 +++++++++++++++++++++++++++++++
 drivers/auxdisplay/line-display.h |  2 ++
 2 files changed, 34 insertions(+)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 915eb5cd96b2..6fd0b870e16a 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -592,5 +592,37 @@ void linedisp_unregister(struct linedisp *linedisp)
 }
 EXPORT_SYMBOL_NS_GPL(linedisp_unregister, "LINEDISP");
 
+static void devm_linedisp_unregister(void *data)
+{
+	struct linedisp *linedisp = data;
+
+	linedisp_unregister(linedisp);
+}
+
+/**
+ * devm_linedisp_register - register a character line display
+ * @dev: device being registered
+ * @linedisp: pointer to character line display structure
+ * @num_chars: the number of characters that can be displayed
+ * @ops: character line display operations
+ *
+ * Managed linedisp_register(). Line display registered with this function will
+ * automatically be unregistered on driver detach.
+ *
+ * Return: zero on success, else a negative error code.
+ */
+int devm_linedisp_register(struct device *dev, struct linedisp *linedisp,
+			   unsigned int num_chars, const struct linedisp_ops *ops)
+{
+	int err;
+
+	err = linedisp_register(linedisp, dev, num_chars, ops);
+	if (err)
+		return err;
+
+	return devm_add_action_or_reset(dev, devm_linedisp_unregister, linedisp);
+}
+EXPORT_SYMBOL_NS_GPL(devm_linedisp_register, "LINEDISP");
+
 MODULE_DESCRIPTION("Character line display core support");
 MODULE_LICENSE("GPL");
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index 36853b639711..cd1b94829f72 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -88,5 +88,7 @@ void linedisp_detach(struct device *dev);
 int linedisp_register(struct linedisp *linedisp, struct device *parent,
 		      unsigned int num_chars, const struct linedisp_ops *ops);
 void linedisp_unregister(struct linedisp *linedisp);
+int devm_linedisp_register(struct device *dev, struct linedisp *linedisp,
+			   unsigned int num_chars, const struct linedisp_ops *ops);
 
 #endif /* LINEDISP_H */
-- 
2.55.0
Re: [PATCH 0/3 1/3] auxdisplay: line-display: add devm_linedisp_register()
Posted by Andy Shevchenko 1 month, 1 week ago
On Wed, Aug 19, 2026 at 08:15:54AM +0530, kr494167@gmail.com wrote:

> Add devm_linedisp_register() to manage character line display registration
> via devres. This simplifies driver cleanup and prevents use-after-free
> bugs when unregistering line displays on driver detach.

This is not enough per se. Copied'n'pasted the reply I made to Rajat who
reported the issue:

-->8----8<--

> > 6. PROPOSED FIX
> >
> > Replace devm_kzalloc with plain kzalloc and tie the container
> > lifetime to the embedded device's refcount:
> >
> > --- a/drivers/auxdisplay/line-display.c
> > +++ b/drivers/auxdisplay/line-display.c
> > @@ (linedisp_release callback)
> >
> >  static void linedisp_release(struct device *dev)
> >  {
> >      struct linedisp *linedisp = to_linedisp(dev);
> > +    struct container *priv = container_of(linedisp, ...);
> >
> >      kfree(linedisp->map);
> >      kfree(linedisp->message);
> >      kfree(linedisp->buf);
> > +    kfree(priv);  /* free container when refcount reaches 0 */
> >  }
> >
> > Each affected driver (img-ascii-lcd, max6959, seg-led-gpio) must
> > change devm_kzalloc to kzalloc for the container struct, and ensure
> > the release function frees it. This aligns the container lifetime
> > with the embedded device refcount.
>
> That won't scale as the device drivers are free to call devm_kzalloc()
> and similar for their private data structures. What we should do is to
> prevent a device from unbinding when one or more files are open (via
> sysfs). TL;DR: downgrading devm_kzalloc() is not an option.

So, the fix as I see it is much more intrusive. The
linedisp_register() should be split to _alloc() and _register() APIs,
and then at least the first one being also wrapped with devm_*() for
users that want this. See how devm_iio_device_alloc() and
devm_iio_device_registers() are implemented.


-- 
With Best Regards,
Andy Shevchenko