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

[Kelsey Skunberg]

On 2020-09-17 18:47:27 , Colin Ian King wrote:
> 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
>

sweet, thank you! and will deffinitely do that change. Thanks for catching it!

-Kelsey

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