From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <53689692.8050304@gmx.at> Date: Tue, 06 May 2014 10:00:18 +0200 From: Manfred Schlaegl MIME-Version: 1.0 To: Peter Hurley , Greg Kroah-Hartman CC: Jiri Slaby , One Thousand Gnomes , linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH 2/2] tty: Fix lockless tty buffer race References: <1399042572-6533-1-git-send-email-peter@hurleysoftware.com> <1399042572-6533-2-git-send-email-peter@hurleysoftware.com> <5363B446.7010101@hurleysoftware.com> In-Reply-To: <5363B446.7010101@hurleysoftware.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: On 2014-05-02 17:05, Peter Hurley wrote: > On 05/02/2014 10:56 AM, Peter Hurley wrote: >> Commit 6a20dbd6caa2358716136144bf524331d70b1e03, >> "tty: Fix race condition between __tty_buffer_request_room and flush_to_ldisc" >> correctly identifies an unsafe race condition between >> __tty_buffer_request_room() and flush_to_ldisc(), where the consumer >> flush_to_ldisc() prematurely advances the head before consuming the >> last of the data committed. For example: >> >> CPU 0 | CPU 1 >> __tty_buffer_request_room | flush_to_ldisc >> ... | ... >> | count = head->commit - head->read >> n = tty_buffer_alloc() | >> b->commit = b->used | >> b->next = n | >> | if (!count) /* T */ >> | if (head->next == NULL) /* F */ >> | buf->head = head->next >> >> In this case, buf->head has been advanced but head->commit may have >> been updated with a new value. >> >> Instead of reintroducing an unnecessary lock, fix the race locklessly. >> Read the commit-next pair in the reverse order of writing, which guarantees >> the commit value read is the latest value written if the head is >> advancing. This is a fine solution! I'll verify this against my previous experimental setup (3.12.x and 3.12.x-rt25), but I dont't expect any problems. >> >> Reported-by: Manfred Schlaegl >> Cc: # 3.12.x+ > > The patch submitted by Manfred notes the commits which introduced the > race [1], but attributes those commits to the 3.11 cycle. Those commits > were merged in the 3.12 cycle. You are right. I'm sorry for this. Regars, Manfred