From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758275AbcAKFkQ (ORCPT ); Mon, 11 Jan 2016 00:40:16 -0500 Received: from mail-pa0-f41.google.com ([209.85.220.41]:35884 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751036AbcAKFkN (ORCPT ); Mon, 11 Jan 2016 00:40:13 -0500 Subject: Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker To: Ben Hutchings , Greg Kroah-Hartman 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> Cc: Jiri Slaby , linux-kernel@vger.kernel.org, stable@vger.kernel.org From: Peter Hurley Message-ID: <5693403B.5040309@hurleysoftware.com> Date: Sun, 10 Jan 2016 21:40:11 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <5692F66D.8060207@hurleysoftware.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. >>> >>> The key difference with the VT's paste_selection() is that is an ioctl, >>> where __speakup_paste_selection() is completely asynch kworker, kicked >>> off from interrupt context. >>> >>> Fixes: 28a821c30688 ("Staging: speakup: Update __speakup_paste_selection() >>> tty (ab)usage to match vt") >>> Cc: >>> >>> Signed-off-by: Peter Hurley >>> --- >>> drivers/staging/speakup/selection.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> 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 work_struct *work) >>> struct tty_ldisc *ld; >>> DECLARE_WAITQUEUE(wait, current); >>> >>> - ld = tty_ldisc_ref_wait(tty); >>> + ld = tty_ldisc_ref(tty); >>> + if (!ld) >>> + return; >>> tty_buffer_lock_exclusive(&vc->port); >>> >>> add_wait_queue(&vc->paste_wait, &wait); >> >> This leaks a reference to the tty. Instead of returning directly, I >> think you need to add a label and goto the tty_kref_put() at the bottom >> of the function. > > Ugh, speakup_paste_selection() is a worse hack than I thought it was. What if the kworker has already been scheduled but not run? Leaky reference anyway. What guarantees that the kref is gettable to begin with and isn't incrementing from 0? This isn't how tty krefs work. I'll fix the patch to drop the kref but this is broken anyway. Regards, Peter Hurley