netdev - RE: [PATCH net 3/3] ice: Add set_termios tty operations handle to GNSS

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: <BN6PR11MB417756CED7AE9DF7C3FA88DCE37F9@BN6PR11MB4177.namprd11.prod.outlook.com>
Date:   Mon, 5 Sep 2022 19:32:44 +0000
From:   "Michalik, Michal" <michal.michalik@...el.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jakub Kicinski <kuba@...nel.org>
CC:     "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "richardcochran@...il.com" <richardcochran@...il.com>,
        "G, GurucharanX" <gurucharanx.g@...el.com>,
        "Jiri Slaby" <jirislaby@...nel.org>,
        Johan Hovold <johan@...nel.org>
Subject: RE: [PATCH net 3/3] ice: Add set_termios tty operations handle to
 GNSS

Hello Greg,

Much thanks for a feedback. Please excuse me for delayed answer, we tried to collect all
the required information before returning to you - but we are still working on it.

Best regards,
M^2

> 
> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@...uxfoundation.org> 
> Sent: Thursday, September 1, 2022 7:46 AM
> To: Jakub Kicinski <kuba@...nel.org>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; davem@...emloft.net; pabeni@...hat.com; edumazet@...gle.com; Michalik, Michal <michal.michalik@...el.com>; netdev@...r.kernel.org; richardcochran@...il.com; G, GurucharanX <gurucharanx.g@...el.com>; Jiri Slaby <jirislaby@...nel.org>; Johan Hovold <johan@...nel.org>
> Subject: Re: [PATCH net 3/3] ice: Add set_termios tty operations handle to GNSS
> 
> On Wed, Aug 31, 2022 at 02:54:39PM -0700, Jakub Kicinski wrote:
> > On Mon, 29 Aug 2022 15:00:49 -0700 Tony Nguyen wrote:
> > > From: Michal Michalik <michal.michalik@...el.com>
> > > 
> > > Some third party tools (ex. ubxtool) try to change GNSS TTY parameters
> > > (ex. speed). While being optional implementation, without set_termios
> > > handle this operation fails and prevents those third party tools from
> > > working.
> 
> What tools are "blocked" by this?  And what is the problem they have
> with just the default happening here?  You are now doing nothing, while
> if you do not have the callback, at least a basic "yes, we accepted
> these values" happens which was intended for userspace to not know that
> there was a problem here.
> 

As I stated in the commit message, the example tool is ubxtool - while trying to
connect to the GPS module the error appreared:
Traceback (most recent call last):

	  File "/usr/local/bin/ubxtool", line 378, in <module>
		io_handle = gps.gps_io(
	  File "/usr/local/lib/python3.9/site-packages/gps/gps.py", line 309, in __init__
		self.ser = Serial.Serial(
	  File "/usr/local/lib/python3.9/site-packages/serial/serialutil.py", line 244, in __init__
		self.open()
	  File "/usr/local/lib/python3.9/site-packages/serial/serialposix.py", line 332, in open
		self._reconfigure_port(force_update=True)
	  File "/usr/local/lib/python3.9/site-packages/serial/serialposix.py", line 517, in _reconfigure_port
		termios.tcsetattr(
	termios.error: (22, 'Invalid argument')
	
Adding this empty function solved the problem.

> > TTY interface in ice driver is virtual and doesn't need any change
> > > on set_termios, so is left empty. Add this mock to support all Linux TTY
> > > APIs.
> 
> "mock"?

Please excuse me if I used the wrong terminology. What I meant by "mock" was
this empty function, which did nothing but satisfied API requirements.

> 
> > > 
> > > Fixes: 43113ff73453 ("ice: add TTY for GNSS module for E810T device")
> > > Signed-off-by: Michal Michalik <michal.michalik@...el.com>
> > > Tested-by: Gurucharan <gurucharanx.g@...el.com> (A Contingent worker at Intel)
> > > Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> > 
> > Please CC GNSS and TTY maintainers on the patches relating to 
> > the TTY/GNSS channel going forward.
> > 
> > CC: Greg, Jiri, Johan
> > 
> > We'll pull in a day or two if there are no objections.
> 
> Please see above, I'd like to know what is really failing here and why
> as forcing drivers to have "empty" functions like this is not good and
> never the goal.

If I should elaborate more on the reproduction, please leave me a note.

> 
> thanks,
> 
> greg k-h
>