linux-kernel - Re: [PATCH 2/5] serial: core: add sysfs attribute to suppress ready signalling on open

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]

Date:   Tue, 1 Dec 2020 09:21:27 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Johan Hovold <johan@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        "Mychaela N . Falconia" <falcon@...ecalypso.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/5] serial: core: add sysfs attribute to suppress ready
 signalling on open

On Mon, Nov 30, 2020 at 08:27:54PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 30, 2020 at 5:42 PM Johan Hovold <johan@...nel.org> wrote:
> >
> > Add a nordy sysfs attribute to suppress raising the modem-control lines
> > on open to signal DTE readiness.
> 
> Why not call it nomctrl ?

That was one of the candidates I rejected.

As I hinted in the cover letter (and patch adding the flag) I chose the
name to match the current termios flags (e.g. HUPCL and NOFLSH).

NOMCTRL is both too general and specific; HUPCL still controls the
modem-control lines on final close. Also, like HUPCL, I wanted a more
general name that can be used for terminal devices which can signal
readiness through other means (e.g. network).

Like the other termios flags it is terse, but once you learn the meaning
it's easy to remember. And I think there's value in keeping the same
name throughout (cf. termios flags and stty).

> > This can be use to prevent undesirable side-effects on open for
> 
> used

Thanks, I'll fix that up before applying or resending

> > applications where the DTR and RTS lines are used for non-standard
> > purposes such as generating power-on and reset pulses.
> 
> ...
> 
> > +static ssize_t nordy_store(struct device *dev, struct device_attribute *attr,
> > +                               const char *buf, size_t count)
> > +{
> > +       struct tty_port *port = dev_get_drvdata(dev);
> > +       unsigned int val;
> > +       int ret;
> > +
> > +       ret = kstrtouint(buf, 0, &val);
> > +       if (ret)
> > +               return ret;
> 
> > +       if (val > 1)
> > +               return -EINVAL;
> 
> Can't we utilise kstrtobool() instead?

I chose not to as kstrtobool() results in a horrid interface. To many
options to do the same thing and you end up with confusing things like
"0x01" being accepted but treated as false (as only the first character
is considered).

Not sure how that ever made it into sysfs code...

The attribute is read back as "0" or "1" and those are precisely the
values that can be written back (well, modulo radix).

It's not relevant in this case, but tight control over the inputs also
allows for extending the range later.

> > +       tty_port_set_nordy(port, val);
> > +
> > +       return count;
> > +}

Johan