Re: [PATCH v2 09/19] staging/speakup: Use tty_ldisc_ref() for paste kworker

[Peter Hurley]

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: <stable@xxxxxxxxxxxxxxx>
>>>
>>> Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
>>> ---
>>>  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