Messages in this thread | | | Date | Fri, 11 Dec 2015 14:34:04 +0000 | From | Alan Cox <> | Subject | Re: 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
| |