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: <YxgwA9bTvdheeZUf@kroah.com>
Date:   Wed, 7 Sep 2022 07:45:39 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     "Michalik, Michal" <michal.michalik@...el.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        "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

On Tue, Sep 06, 2022 at 08:55:59PM +0000, Michalik, Michal wrote:
> Greg,
> 
> Thanks - answer inline.

As is required, no need to put this on your emails, as top-posting is
not allowed :)

> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@...uxfoundation.org> 
> > Sent: Tuesday, September 6, 2022 8:16 AM
> > To: Michalik, Michal <michal.michalik@...el.com>
> > Cc: Jakub Kicinski <kuba@...nel.org>; Nguyen, Anthony L <anthony.l.nguyen@...el.com>; davem@...emloft.net; pabeni@...hat.com; edumazet@...gle.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

Please fix your email client to not do this.

> > > Adding this empty function solved the problem.
> > 
> > That seems very wrong, please work to fix this by NOT having an empty
> > function like this as it should not be required.
> > 
> 
> I don't get one thing, though. You are saying, it "seem" wrong and that
> "should not" be required but I observe different behavior. I have prepared
> a very simple code to reproduce the issue:
> 	#include <termios.h>
> 	#include <unistd.h>
> 	#include <stdio.h>
> 	#include <fcntl.h>
> 	#include <errno.h>
> 
> 	int main()
> 	{
> 		struct termios tty;
> 		int fd;
> 		
> 		fd = open("/dev/ttyGNSS_0300", O_RDWR | O_NOCTTY | O_SYNC);
> 
> 		if (fd < 0) {
> 				printf("Error - TTY not open.\n");
> 				return -1;
> 		}
> 				
> 		if (tcgetattr (fd, &tty) != 0) {
> 			printf("Error on get - errno=%i\n", errno);
> 			return -1;
> 		}
> 		tty.c_cflag |= CS8; // try to set 8 data bits 
> 		if (tcsetattr(fd, TCSANOW, &tty) != 0) {
> 			printf("Error on set - errno=%i\n", errno);
> 			return -1;
> 		}
> 
> 		close(fd);
> 		printf("Done.\n");
> 	}
> 
> In this case, when I don't satisfy this API, I get an errno 22.

You get the error on the first get or the set?

> If add this
> empty function and therefore implement the full API it works as expected (no
> error). In our case no action is needed, therefore we have an empty function.
> At the moment, I'm not sure how I should fix it other way - since no action
> on HW is neccessary.

This should not be needed as I thought the default would be "just ignore
this", but maybe not.  Can you look into this problem please and figure
out why this is required and fix that up?

> Of course in the meantime we are working on investigating if we can easily
> align to existing GNSS interface accroding to community suggestions. Still,
> we believe that this fix is solving the problem at the moment. 

Let's fix the root problem here, not paper over it.

thanks,

greg k-h