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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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