linux-kernel - Re: [PATCH 0/3] USB-Serial serdev support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251024092738.zao47ehvzckkrsf3@pengutronix.de>
Date: Fri, 24 Oct 2025 11:27:38 +0200
From: Marco Felsch <m.felsch@...gutronix.de>
To: Johan Hovold <johan@...nel.org>
Cc: Rob Herring <robh@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 0/3] USB-Serial serdev support

On 25-10-24, Johan Hovold wrote:
> On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> > On 25-10-23, Johan Hovold wrote:
> > > On Thu, Mar 13, 2025 at 08:40:44PM +0100, Marco Felsch wrote:
> > > > On 25-03-11, Johan Hovold wrote:
> > > > > On Tue, Sep 17, 2024 at 06:49:48AM +0200, Marco Felsch wrote:
> > > > > > On 24-09-09, Johan Hovold wrote:
> 
> > > > > > > First, as the commit message you refer to below explain, we need some
> > > > > > > way to describe multiport controllers. Just dropping the 'serial' node
> > > > > > > does not make that issue go away.
> 
> > > There are other USB serial devices that support multiple ports over a
> > > single USB interface. The DT bindings need to account for that case as
> > > well, and that probably means we should not be describing the interfaces
> > > at all but rather the physical ports.
> > 
> > Ah okay, I wasn't even aware that this possible too. However this is DT
> > description and another topic.
> 
> It's still one of the issues that need to addressed.

Yes but this shouldn't be an issue with this patchset. So far the
smallest DT-describale USB entities are the interfaces.

Additional support needs to be added if there are devices which need a
more fine-grained description. I can't implement this since I don't have
access to such devices. That beeing said, my patchset doesn't break such
devices because in that case these devices simply can't be described as
serdev device within the DT and the ttyUSBx devices are exposed to
userspace.

If one wants to add the support for it, the support can surely be added
afterwards too in a backward compatible manner.

> > > > > > > Second, and more importantly, you do not address the main obstacle for
> > > > > > > enabling serdev for USB serial which is that the serdev cannot handle
> > > > > > > hotplugging.
> 
> > > You will also see the following kind of warnings in the logs:
> > > 
> > > ttyUSB ttyUSB0: tty_hangup: tty->count(1) != (#fd's(0) + #kopen's(0))
> > > ttyUSB ttyUSB0: tty_port_close_start: tty->count = 1 port count = 0
> > > 
> > > which are due to the fact that serdev does not support hangups which are
> > > used during teardown of USB serial ports.
> > 
> > IIRC I added the following patch to solve this:
> > 
> >  - [PATCH 1/3] serdev: ttyport: make use of tty_kopen_exclusive
> > 
> > Sorry for not remembering the details since this conversation/patchset
> > is quite old but still one of our top prios.
> 
> That suppresses the first warning but doesn't address the underlying
> issue (that hangups are built around file handles which serdev does not
> use). And you will still see the second one when the serdev driver tries
> to close the already hung up port during deregistration.

Can you please elaborate how I can check this? I'm not aware of any
warning yet, but I only tested the hot-(un)plug. If I got your right, I
should see the issue once I unload the serdev driver, right?

> Also, that commit message needs to more work since you don't really
> motivate why you think it's needed (e.g. as serdev ports can't be shared
> with user space).

Maybe it needs some adaptions but:

| The purpose of serdev is to provide kernel drivers for particular serial
| device, serdev-ttyport is no exception here. Make use of the
| tty_kopen_exclusive() funciton to mark this tty device as kernel
| internal device.

the last sentence should address your point that serdev ports can't be
shared with user-space.

> If it's just about suppressing the warning you could possibly just have
> set that new flag.

Which new flag? As I have written in my commit message: "Make use of ...
to mark this tty device as kernel internal device". I thought this was
the purpose of tty_kopen_exclusive().

Regards,
  Marco


> 
> Johan
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |