linux-kernel - Re: [PATCH] n_tty: Fix unordered accesses to lockless read buffer

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:	Sun, 04 Jan 2015 14:44:46 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	Christian Riesch <christian.riesch@...cron.at>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>, linux-serial@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	stable <stable@...r.kernel.org>
Subject: Re: [PATCH] n_tty: Fix unordered accesses to lockless read buffer

On 01/01/2015 06:00 AM, Christian Riesch wrote:
> Peter,
> 
> Thank you for this patch! Unfortunately I had not much time for this
> since my last patch submission, so I am happy that someone continued
> this work.
> 
> I have a few comments/questions, please see below.
> 
> On Tue, Dec 30, 2014 at 1:05 PM, Peter Hurley <peter@...leysoftware.com> wrote:
>> Add commit_head buffer index, which the producer-side publishes
>> after input processing. This ensures the consumer-side observes
>> correctly-ordered writes in raw mode
> 
> I understand that the commit_head reduces the number of memory
> barriers and makes some things easier. But what is so special about
> raw mode that requires the introduction of commit_head?

commit_head is simply the read_head after each received buffer is
processed by the input worker. In this context, I meant 'raw mode' as
any non-canonical mode, ie., any mode in which copy_from_read_buf()
is used by the reader. I'll replace 'raw' with 'non-canonical' instead.

>> (ie., the buffer data is
>> written before the buffer index is advanced).
>>
>> Further, remove read_cnt() and expand inline, using ACCESS_ONCE()
> 
> "ACCESS_ONCE() and memory barriers"?

This portion of the changelog refers only to read_cnt(), for which
memory barriers are not required.

But, while writing the explanatory fragment [1], I realized that
input_available_p() must load the most recent relevant head index
(either canon_head or commit_head based on the mode) because
it may conclude there is no more input _and_ then observe an end-of-file
condition. So I need to fix this up to do the smp_load_acquire() in
input_available_p() and ACCESS_ONCE() in the *_copy_from_read_buf().

Regards,
Peter Hurley

[1]
Strictly speaking, the ACCESS_ONCE()'s are optimizations. Neither the
producer nor consumer require the most recent 'opposed' index; if the
compiler elided the opposed index load, instead reusing an existing load

--
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/