Re: [PATCH v4 1/4] tty: serdev: Add mutex lock

[Markus Probst <markus.probst@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

On Sat, 2026-10-17 at 09:53 +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 16, 2026 at 06:00:32PM +0000, Markus Probst wrote:
> > Besides more predictable behaviour, this allows for several hardened
> > behaviour changes:
> > 
> > Return -EALREADY in `serdev_device_open` if the device is already open
> > instead of causing undefined behaviour.
> > 
> > Allow calling `serdev_device_close`, even if the device is already
> > closed instead of causing a null pointer dereference.
> > 
> > If the device is left open by the driver after remove, close it and warn
> > instead of leaving it in a invalid state.
> > 
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> >  drivers/tty/serdev/core.c           | 10 +++++---
> >  drivers/tty/serdev/serdev-ttyport.c | 46 +++++++++++++++++++++++++++++++------
> >  include/linux/serdev.h              |  2 +-
> >  3 files changed, 47 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index 7500efcdfc21..77e8e1d4d2a6 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -142,6 +142,11 @@ void serdev_device_remove(struct serdev_device *serdev)
> >  	struct serdev_controller *ctrl = serdev->ctrl;
> >  
> >  	device_unregister(&serdev->dev);
> > +
> > +	/* Warn if driver did not close the serial device. */
> > +	if (ctrl->ops->close && WARN_ON(ctrl->ops->close(ctrl)))
> 
> I understand the want to give a big "splat" if this happens, but if it
> does somehow, you just crashed the box due to panic-on-warn, which will
> result in a CVE being given :)
> 
> So, if this ever can happen, we should recover like you do:
> 
> > +		pm_runtime_put(&ctrl->dev);
> 
> And you can warn about it in the kernel log, but just not with WARN_ON()
> please.
To be honest, I forgot about this review and sent v5 without taking it
into account.


Anyway, I looked into documentation.

"WARN*() is intended for unexpected, this-should-never-happen
situations. [...] However, the existence of panic_on_warn users is not
a valid reason to avoid the judicious use WARN*()." [1]

I think this is a valid case.

It is an event that should never happen, and it might cause a use-
after-free vulnerability on data reception.

Thanks
- Markus Probst

[1] https://docs.kernel.org/process/coding-style.html


> 
> thanks,
> 
> greg k-h

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]