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] [day] [month] [year] [list]
Date:	Wed, 03 Sep 2008 12:00:26 +0200
From:	Denis Joseph Barrow <D.Barow@...ion.com>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>, linux-kernel@...r.kernel.org
Subject: Re: /drivers/char/n_tty.c drops characters

Hi Alan,
Sorry for wasting your valuable time.
There are tty_throttle & tty_unthrottle functions
should if working properly do everything I want.

Alan Cox wrote:
>> Maybe my interpretation of this statement is incorrect but
>> is Alan implying that ttys will no longer drop charaters ? 
>> if this is what Alan is saying it is simply not true but it can be implemented.
> 
> The current queue code will not drop characters queued to the line
> discipline unless we are in extreme circumstances [64K queued on this
> tty, out of memory]
> 
>> My understanding of this code might be imperfect but this really looks
>> likes this blindly copies into a ring buffer of N_TTY_BUF_SIZE &
>> doesn't return any indication an attempt to copy a tty buffer of larger than N_TTY_BUF_SIZE
>> & overflows the buffer before the task activated by the code at the bottom of n_tty_receive_buf
> 
> There is no check needed. See flush_to_ldisc
> 
>             if (count > tty->receive_room)
>                     count = tty->receive_room;
> 
> So if we are dropping characters on this path it means
> tty->receive_room is either getting set too large or the n_tty driver is
> shrinking it by more than the bytes it gets a some point.
> 
> Most of the network drivers actually just set it to "send everything" as
> they want overruns on the serial side to drop bytes to get proper TCP
> queueing but n_tty does try to manage the value.
> 
>> My main complaints are
>> 1) there is no mechanism for informing the tty layer that not all the
>> characters are copied from the tty layer to the line discipline.
> 
> The tty layer doesn't need to know. What will the tty driver do if it
> finds this - queue bytes ? We already queue bytes in the core code now
> for up to 64K. Flow control ? - it is late for flow control at that point,
> and the core code already provides flow control callbacks.
> 
>> 2) The use of a a ring buffer in the line discipline,
>> we are memcpying at least twice in the kernel before passing the
>> buffer back to userland.
> 
> For n_tty this really does not worry me. n_tty is not a performance
> critical path for any use I know of. For the ppp layer we do a copy from
> the tty buffers to the socket buffers and we have to do that to unpack
> the async padding. That one is the one that bothers me more. I'm willing
> to be enlightened however.
> 
> We do try to keep all the transfers cache-hot. In theory you could tweak
> the ldisc code so that the tty_buffer object ownership is transferred to
> the ldisc which then frees the buffer back when it is done. That may have
> possibilities.
> 
>> 3) There is no mechanism for tty_io.c of informing the
>> char driver which is feeding the tty that it can
>> release flow control & start feeding read buffers
>> to the device hardware again to restart io. 
> 
> driver->ops->unthrottle()
> 
> I am not 100% certain we call throttle early enough and unthrottle
> at times appropriate for devices with large internal buffers like some of
> the USB hardware but those are points that can be adjusted.


-- 
best regards,
D.J. Barrow
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ