linux-kernel - Re: [PATCH 3.12 16/33] isdn/gigaset: reset tty->receive_room when attaching ser

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: <56002B48.7090008@imap.cc>
Date:	Mon, 21 Sep 2015 18:07:36 +0200
From:	Tilman Schmidt <tilman@...p.cc>
To:	Peter Hurley <peter@...leysoftware.com>
Cc:	Jiri Slaby <jslaby@...e.cz>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	Linux kernel mailing list <linux-kernel@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 3.12 16/33] isdn/gigaset: reset tty->receive_room when
 attaching ser_gigaset

Am 21.09.2015 um 15:13 schrieb Peter Hurley:
> On 09/18/2015 08:38 AM, Tilman Schmidt wrote:
>> Am 17.09.2015 um 20:13 schrieb Peter Hurley:

[...]

>>> So for example, if you manually set N_PPP (as if by user error)
>>
>> User error wouldn't suffice, as the LD would get reset to N_TTY when the
>> serial device is closed. You would have to write a program that
>> deliberately switched the LD first to N_PPP and then to N_GIGASET_M101
>> without closing the device in between.
> 
> ???
> 
> The tool you authored will do it from the command line
> 
> $ ldattach PPP /dev/ttyS1
> $ ldattach GIGASET_M101 /dev/ttyS1
> 
> Note that nothing here closes the serial device 'in between', and
> the tty core has switched directly from PPP to GIGASET_M101.
> n_tty->receive_room is now 64K.

Indeed it does. I stand corrected. The possibility of running ldattach a
second time without terminating the first instance didn't occur to me.

> Please add switching from line disciplines other than N_TTY to your
> regression testing.

I don't do regression tests for the driver anymore since I stepped down
as a maintainer, so that would be up to the present maintainer of
ser_gigaset. But I see no reason for that. As I already explained, N_TTY
is the only problematic case.

>>> and then set this line discipline, tty->receive_room will be 64K, not 4K.
>>
>> That wouldn't affect the operation of ser_gigaset,
> 
> I've explained this before to you, but here it is again:
> 
> tty->receive_room announces the maximum amt of data the line discipline
> can accept from tty core with each call to its receive_buf() method (for
> line disciplines that don't provide flow control).
> 
> If the line discipline sets ->receive_room to 64K but can only handle
> 8K (as in the case of GIGASET_M101), then data loss should be the expected
> result.

If you'd care to look at the actual code you'd notice that it truly
won't make any difference. The receive_buf() method of ser_gigaset is
prepared to drop data and log an error when its receive buffer
overflows, no matter how big the block of data passed from tty core is.
The only difference a smaller ->receive_room value might possibly make
is to distribute the overflowing data to more receive_buf() calls.

(Note that the Gigaset M101 device operates at 115200 bits/sec max. so
it takes at least 700 msecs to transmit 8k bytes. If we ever get into a
situation where tty core actually accumulates more than that amount of
data before forwarding it to GIGASET_M101 then we have a more serious
problem anyway.)

Again, I won't oppose applying this patch to stable releases before
3.10. I just don't see the need, so it would be up to you to advocate
such a request.

-- 
Tilman Schmidt                              E-Mail: tilman@...p.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)