From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754417AbbLKOe3 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 11 Dec 2015 09:34:29 -0500
Received: from lxorguk.ukuu.org.uk ([81.2.110.251]:51946 "EHLO
	lxorguk.ukuu.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753769AbbLKOe2 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 11 Dec 2015 09:34:28 -0500
Date: Fri, 11 Dec 2015 14:34:04 +0000
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jslaby@suse.com>, LKML <linux-kernel@vger.kernel.org>,
        Paul Fulghum <paulkf@microgate.com>, Arnd Bergmann <arnd@arndb.de>,
        syzkaller <syzkaller@googlegroups.com>,
        Kostya Serebryany <kcc@google.com>,
        Alexander Potapenko <glider@google.com>,
        Sasha Levin <sasha.levin@oracle.com>,
        Eric Dumazet <edumazet@google.com>
Subject: Re: tty: WARNING in n_hdlc_tty_read
Message-ID: <20151211143404.73e0540e@www.etchedpixels.co.uk>
In-Reply-To: <CACT4Y+aK4RaeXaEs51cYtUzytsDD4_QJj_U7WmAHQC5z6jNr7Q@mail.gmail.com>
References: <CACT4Y+aK4RaeXaEs51cYtUzytsDD4_QJj_U7WmAHQC5z6jNr7Q@mail.gmail.com>
X-Mailer: Claws Mail 3.12.0 (GTK+ 2.24.28; x86_64-redhat-linux-gnu)
Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAFVBMVEWysKsSBQMIAwIZCwj///8wIhxoRDXH9QHCAAABeUlEQVQ4jaXTvW7DIBAAYCQTzz2hdq+rdg494ZmBeE5KYHZjm/d/hJ6NfzBJpp5kRb5PHJwvMPMk2L9As5Y9AmYRBL+HAyJKeOU5aHRhsAAvORQ+UEgAvgddj/lwAXndw2laEDqA4x6KEBhjYRCg9tBFCOuJFxg2OKegbWjbsRTk8PPhKPD7HcRxB7cqhgBRp9Dcqs+B8v4CQvFdqeot3Kov6hBUn0AJitrzY+sgUuiA8i0r7+B3AfqKcN6t8M6HtqQ+AOoELCikgQSbgabKaJW3kn5lBs47JSGDhhLKDUh1UMipwwinMYPTBuIBjEclSaGZUk9hDlTb5sUTYN2SFFQuPe4Gox1X0FZOufjgBiV1Vls7b+GvK3SU4wfmcGo9rPPQzgIabfj4TYQo15k3bTHX9RIw/kniir5YbtJF4jkFG+dsDK1IgE413zAthU/vR2HVMmFUPIHTvF6jWCpFaGw/A3qWgnbxpSm9MSmY5b3pM1gvNc/gQfwBsGwF0VCtxZgAAAAASUVORK5CYII=
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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