[SRU][X][B][F][G][PATCH 1/1] tty: hvcs: Don't NULL tty->driver_data until hvcs_cleanup()

[Colin Ian King]

On 17/09/2020 18:41, Kelsey Skunberg wrote:
> On 2020-09-16 10:19:39 , Colin Ian King wrote:
>> On 15/09/2020 19:56, patricia.domingues at canonical.com wrote:
>>> From: Tyrel Datwyler <tyreld at linux.ibm.com>
>>>
>>> BugLink: https://bugs.launchpad.net/bugs/1892546
>>>
>>> The code currently NULLs tty->driver_data in hvcs_close() with the
>>> intent of informing the next call to hvcs_open() that device needs to be
>>> reconfigured. However, when hvcs_cleanup() is called we copy hvcsd from
>>> tty->driver_data which was previoulsy NULLed by hvcs_close() and our
>>> call to tty_port_put(&hvcsd->port) doesn't actually do anything since
>>> &hvcsd->port ends up translating to NULL by chance. This has the side
>>> effect that when hvcs_remove() is called we have one too many port
>>> references preventing hvcs_destuct_port() from ever being called. This
>>> also prevents us from reusing the /dev/hvcsX node in a future
>>> hvcs_probe() and we can eventually run out of /dev/hvcsX devices.
>>>
>>> Fix this by waiting to NULL tty->driver_data in hvcs_cleanup().
>>>
>>> Fixes: 27bf7c43a19c ("TTY: hvcs, add tty install")
>>> Signed-off-by: Tyrel Datwyler <tyreld at linux.ibm.com>
>>> Link: https://lore.kernel.org/r/20200820234643.70412-1-tyreld@linux.ibm.com
>>> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>>> (cherry picked from commit 63ffcbdad738e3d1c857027789a2273df3337624)
>>> Signed-off-by: Patricia Domingues <patricia.domingues at canonical.com>
>>> ---
>>>  drivers/tty/hvc/hvcs.c | 14 +++++++-------
>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
>>> index 5997b1731111..cba662c50f91 100644
>>> --- a/drivers/tty/hvc/hvcs.c
>>> +++ b/drivers/tty/hvc/hvcs.c
>>> @@ -1232,13 +1232,6 @@ static void hvcs_close(struct tty_struct *tty, struct file *filp)
>>>  
>>>  		tty_wait_until_sent(tty, HVCS_CLOSE_WAIT);
>>>  
>>> -		/*
>>> -		 * This line is important because it tells hvcs_open that this
>>> -		 * device needs to be re-configured the next time hvcs_open is
>>> -		 * called.
>>> -		 */
>>> -		tty->driver_data = NULL;
>>> -
>>>  		free_irq(irq, hvcsd);
>>>  		return;
>>>  	} else if (hvcsd->port.count < 0) {
>>> @@ -1254,6 +1247,13 @@ static void hvcs_cleanup(struct tty_struct * tty)
>>>  {
>>>  	struct hvcs_struct *hvcsd = tty->driver_data;
>>>  
>>> +	/*
>>> +	 * This line is important because it tells hvcs_open that this
>>> +	 * device needs to be re-configured the next time hvcs_open is
>>> +	 * called.
>>> +	 */
>>> +	tty->driver_data = NULL;
>>> +
>>>  	tty_port_put(&hvcsd->port);
>>>  }
>>>  
>>>
>>
>> Sane testing, fix looks good.
>>
>> The cherry pick is from linux-next though, I can't find that sha in linux.
>>
>> Colin
>>
> 
> Hey Colin, 
> 
> Is this suppose to be an ACK or just a comment? Pending this is fully
> reviewed, I'll change the cherry pick line while applying. Thanks! 

It's now an ACK since I know the cherry pick like will be fixed up :-)

thanks,

Colin

> 
> -Kelsey 
> 
>> -- 
>> kernel-team mailing list
>> kernel-team at lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team