lkml.org 
[lkml]   [2015]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: tty: WARNING in n_hdlc_tty_read
On Fri, 11 Dec 2015 14:44:00 +0100
Dmitry Vyukov <dvyukov@google.com> wrote:

> Hello,
>
> I am hitting the following WARNING on commit
> aa53685549a2cfb5f175b0c4a20bc9aa1e5a1b85 (Dec 8):

What surprises me is that the warnings didn't trigger for whoever
tested the code.

It also looks to me like the claim in the commit that

" 1. neither BKL or tty mutex are required for correct operation"

is probably not entirely true (though close).

Consider the case where two reads occur in parallel. In that situation
nothing protects hdlc->rx_free_buf_list.count during the decision whether
to kfree or n_hdlc_buf_put.

The more specific problem you are hitting I think those is the attempt to
make use of set_current_state and add_wait_queue directly.

After we set the task to "uninterruptible sleep" we dequeue a frame which
is fine and correctly locked, but then call copy_to_user which can block.
No can do.


My first thought would be to rip out all the raw wait queue messing about
and replace the lot with something slightly more 21st century. Something
a bit like


if (file->f_flags & O_NONBLOCK)
rbuf = n_hdlc_get(...)
else
wait_event_interruptible(&tty->read_wait, (rbuf =
n_hdlc_get(&n_hdlc->rx_buf_list)) != NULL);

if (rbuf)
...

if (signal_pending(current))
...

ought to get close - but that doesn't fit the list.count race.

Alan


\
 
 \ /
  Last update: 2015-12-11 15:41    [W:0.062 / U:0.892 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site