Re: [PATCH 2/2] tty: Fix lockless tty buffer race

From: Manfred Schlaegl
Date: Tue May 06 2014 - 04:02:26 EST


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 <manfred.schlaegl@xxxxxx>
>> Cc: <stable@xxxxxxxxxxxxxxx> # 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/