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

Kelsey Skunberg kelsey.skunberg at canonical.com
Thu Sep 17 17:41:18 UTC 2020


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! 

-Kelsey 

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



More information about the kernel-team mailing list