From: Tyrel Datwyler <tyreld@linux.ibm.com> To: gregkh@linuxfoundation.org Cc: Jiri Slaby <jirislaby@kernel.org>, Jason Yan <yanaijie@huawei.com>, Joe Perches <joe@perches.com>, "open list:HYPERVISOR VIRTUAL CONSOLE DRIVER" <linuxppc-dev@lists.ozlabs.org>, open list <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] tty: hvcs: Don't NULL tty->driver_data until hvcs_cleanup() Date: Fri, 21 Aug 2020 14:08:46 -0700 [thread overview] Message-ID: <096400db-d454-d254-aa30-437352360f7f@linux.ibm.com> (raw) In-Reply-To: <20200820234643.70412-1-tyreld@linux.ibm.com> On 8/20/20 4:46 PM, Tyrel Datwyler wrote: > 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(). I just realized I neglected a Fixes tag. Fixes: 27bf7c43a19c ("TTY: hvcs, add tty install") -Tyrel > > Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.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 55105ac38f89..509d1042825a 100644 > --- a/drivers/tty/hvc/hvcs.c > +++ b/drivers/tty/hvc/hvcs.c > @@ -1216,13 +1216,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) { > @@ -1237,6 +1230,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); > } > >
WARNING: multiple messages have this Message-ID (diff)
From: Tyrel Datwyler <tyreld@linux.ibm.com> To: gregkh@linuxfoundation.org Cc: Joe Perches <joe@perches.com>, "open list:HYPERVISOR VIRTUAL CONSOLE DRIVER" <linuxppc-dev@lists.ozlabs.org>, Jiri Slaby <jirislaby@kernel.org>, open list <linux-kernel@vger.kernel.org>, Jason Yan <yanaijie@huawei.com> Subject: Re: [PATCH] tty: hvcs: Don't NULL tty->driver_data until hvcs_cleanup() Date: Fri, 21 Aug 2020 14:08:46 -0700 [thread overview] Message-ID: <096400db-d454-d254-aa30-437352360f7f@linux.ibm.com> (raw) In-Reply-To: <20200820234643.70412-1-tyreld@linux.ibm.com> On 8/20/20 4:46 PM, Tyrel Datwyler wrote: > 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(). I just realized I neglected a Fixes tag. Fixes: 27bf7c43a19c ("TTY: hvcs, add tty install") -Tyrel > > Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.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 55105ac38f89..509d1042825a 100644 > --- a/drivers/tty/hvc/hvcs.c > +++ b/drivers/tty/hvc/hvcs.c > @@ -1216,13 +1216,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) { > @@ -1237,6 +1230,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); > } > >
next prev parent reply other threads:[~2020-08-21 21:09 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-20 23:46 [PATCH] tty: hvcs: Don't NULL tty->driver_data until hvcs_cleanup() Tyrel Datwyler 2020-08-20 23:46 ` Tyrel Datwyler 2020-08-21 21:08 ` Tyrel Datwyler [this message] 2020-08-21 21:08 ` Tyrel Datwyler 2020-09-14 5:35 ` Jiri Slaby 2020-09-14 5:35 ` Jiri Slaby
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=096400db-d454-d254-aa30-437352360f7f@linux.ibm.com \ --to=tyreld@linux.ibm.com \ --cc=gregkh@linuxfoundation.org \ --cc=jirislaby@kernel.org \ --cc=joe@perches.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=yanaijie@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.