From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759376AbcAKKiF (ORCPT ); Mon, 11 Jan 2016 05:38:05 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:53392 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758191AbcAKKiD (ORCPT ); Mon, 11 Jan 2016 05:38:03 -0500 Message-ID: <1452508667.2435.110.camel@decadent.org.uk> Subject: Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker From: Ben Hutchings To: Peter Hurley , Greg Kroah-Hartman Cc: Jiri Slaby , linux-kernel@vger.kernel.org, stable@vger.kernel.org Date: Mon, 11 Jan 2016 10:37:47 +0000 In-Reply-To: <5693403B.5040309@hurleysoftware.com> References: <1448660356-6328-1-git-send-email-peter@hurleysoftware.com> <1452400870-6005-1-git-send-email-peter@hurleysoftware.com> <1452400870-6005-10-git-send-email-peter@hurleysoftware.com> <1452467777.2435.66.camel@decadent.org.uk> <5692F66D.8060207@hurleysoftware.com> <5693403B.5040309@hurleysoftware.com> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-3VXcrRMT4ZoN49N3WDBA" X-Mailer: Evolution 3.18.3-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 192.168.4.247 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-3VXcrRMT4ZoN49N3WDBA Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2016-01-10 at 21:40 -0800, Peter Hurley wrote: > On 01/10/2016 04:25 PM, Peter Hurley wrote: > > On 01/10/2016 03:16 PM, Ben Hutchings wrote: > > > On Sat, 2016-01-09 at 20:41 -0800, Peter Hurley wrote: > > > > As the function documentation for tty_ldisc_ref_wait() notes, it is > > > > only callable from a tty file_operations routine; otherwise there > > > > is no guarantee the ref won't be NULL. > > > >=20 > > > > The key difference with the VT's paste_selection() is that is an io= ctl, > > > > where __speakup_paste_selection() is completely asynch kworker, kic= ked > > > > off from interrupt context. > > > >=20 > > > > Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_sele= ction() > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0tty (ab)usage to match vt= ") > > > > Cc: > > > >=20 > > > > Signed-off-by: Peter Hurley > > > > --- > > > > =C2=A0drivers/staging/speakup/selection.c | 4 +++- > > > > =C2=A01 file changed, 3 insertions(+), 1 deletion(-) > > > >=20 > > > > diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/= speakup/selection.c > > > > index aa5ab6c..86c0b9a 100644 > > > > --- a/drivers/staging/speakup/selection.c > > > > +++ b/drivers/staging/speakup/selection.c > > > > @@ -142,7 +142,9 @@ static void __speakup_paste_selection(struct wo= rk_struct *work) > > > > =C2=A0 struct tty_ldisc *ld; > > > > =C2=A0 DECLARE_WAITQUEUE(wait, current); > > > > =C2=A0 > > > > - ld =3D tty_ldisc_ref_wait(tty); > > > > + ld =3D tty_ldisc_ref(tty); > > > > + if (!ld) > > > > + return; > > > > =C2=A0 tty_buffer_lock_exclusive(&vc->port); > > > > =C2=A0 > > > > =C2=A0 add_wait_queue(&vc->paste_wait, &wait); > > >=20 > > > This leaks a reference to the tty.=C2=A0=C2=A0Instead of returning di= rectly, I > > > think you need to add a label and goto the tty_kref_put() at the bott= om > > > of the function. > >=20 > > Ugh, speakup_paste_selection() is a worse hack than I thought it was. >=20 > What if the kworker has already been scheduled but not run? Leaky referen= ce > anyway. I don't think so - the cmpxchg() should ensure that only one paste is outstanding. > What guarantees that the kref is gettable to begin with and isn't increme= nting > from 0? Surely it's a bug in the caller of speakup_paste_selection() if it doesn't hold a reference? Ben. > This isn't how tty krefs work. >=20 > I'll fix the patch to drop the kref but this is broken anyway. >=20 > Regards, > Peter Hurley >=20 --=20 Ben Hutchings Q. Which is the greater problem in the world today, ignorance or apathy? A. I don't know and I couldn't care less. --=-3VXcrRMT4ZoN49N3WDBA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUAVpOF++e/yOyVhhEJAQp8rw/+IFXIR9/1JybdKmjLJcMO/CDBoAJEf6Sp r2vTy8qIswkg3Dswbuluo0UcGXAte17q3adUD4JQeLER1qu3huqnmmFtA+DR+SxC VHLbeOHpPZL1Q96obTfDLk4nnfST7bXu2rl0was78m0wjc3lbWOrMZCyCiIWinRv A7/XESVuBFvFX0b062Aobp5vADZdEbu12O68oH6l/DzgVfDLyvWgNoCw4yjNyaOT B38jnhCwtXPU41CgJqYNCXNozx1t4k86EgX4SdBcjo0OnyCNYChGStfm1Pf8ZldK N1giR6TvNls2XwaT30tFj4lXREPQ6kIrx7IJfJCQDF4ihEys2cRMBzjnQ4w7TdkQ 960RwapJOIvNvDx/jQorUmfsmdPPQqrYS51cEJdJsDrHD01VWSnfUA9NUpMzlG5K sPr5p+OUvxSoUEQAYMTq5ESo09SzzHxnpDjabUFnLM9QsjRclHwQwxv6b2Hj+3Z+ ACKlEHYQZ+wzCR7U72adK803K0vnxlq/egJc9TVR9HQV5BIrZUUJ8EBMJfHAQScn yNMw9uelsD3YD/h/6uoDjEiY1iAbuE9kQwRgqEzbSKa+PQpE8OuZp1iSCVgF2tow u5Bx3WGg8wIb2ZeMJlswQVCLqEYH998iw4WLrNSlW1BJkdWaY11awCVHa1apRMEw HiZzNoy0Y+c= =Fs0B -----END PGP SIGNATURE----- --=-3VXcrRMT4ZoN49N3WDBA--