All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] TTY: memory leaks patchset
@ 2012-11-15  8:49 Jiri Slaby
  2012-11-15  8:49 ` [PATCH 1/9] TTY: isicom, stop using port->tty Jiri Slaby
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

I introduced severe memory leaks by (still -next) commit "TTY: move
tty buffers to tty_port", especially in the pty driver. I did not
realize that tty_port is not destroyed as I had imagined. Most of the
drivers simply do not use the tty_port-reference-counted model.

In this series, some are converted to reference counting -- those
which already do reference counting with their own kref. The rest is
just forced to call a newly added function which properly destroys the
tty_port.

Plus there is one patch to fix isicom accessing unsafe tty pointer.

This series should go upstream regardless "TTY: move tty buffers to
tty_port" if we decide to revert that commit due to the reported
warnings I cannot reproduce and neither catch the root cause. At least
while trying to reproduce those I found the leaks fixed here.

Jiri Slaby (9):
  TTY: isicom, stop using port->tty
  TTY: pty, fix tty buffers leak
  ISDN: capi, use kref from tty_port
  MMC: sdio_uart, remove unused member from sdio_uart_port
  MMC: sdio, use kref from tty_port
  TTY: n_gsm, use kref from tty_port
  TTY: introduce tty_port_destroy
  TTY: isicom, fix tty buffers memory leak
  TTY: call tty_port_destroy in the rest of drivers

 arch/alpha/kernel/srmcons.c             |  5 ++++-
 arch/ia64/hp/sim/simserial.c            |  1 +
 arch/m68k/emu/nfcon.c                   |  6 ++++--
 arch/parisc/kernel/pdc_cons.c           |  5 +++--
 arch/um/drivers/line.c                  |  2 ++
 arch/xtensa/platforms/iss/console.c     |  1 +
 drivers/char/pcmcia/synclink_cs.c       |  5 ++++-
 drivers/char/ttyprintk.c                |  4 +++-
 drivers/isdn/capi/capi.c                | 34 ++++++++++++++++-----------------
 drivers/isdn/gigaset/common.c           | 10 ++++++----
 drivers/isdn/i4l/isdn_tty.c             |  4 ++++
 drivers/misc/pti.c                      |  7 +++++--
 drivers/mmc/card/sdio_uart.c            | 24 +++++++++++------------
 drivers/net/usb/hso.c                   |  5 +++--
 drivers/s390/char/con3215.c             |  1 +
 drivers/s390/char/sclp_tty.c            |  4 +++-
 drivers/s390/char/sclp_vt220.c          |  2 ++
 drivers/s390/char/tty3270.c             |  2 ++
 drivers/staging/ccg/u_serial.c          |  5 ++++-
 drivers/staging/dgrp/dgrp_specproc.c    |  2 ++
 drivers/staging/dgrp/dgrp_tty.c         |  4 +++-
 drivers/staging/ipack/devices/ipoctal.c |  2 ++
 drivers/tty/amiserial.c                 |  2 ++
 drivers/tty/bfin_jtag_comm.c            |  6 ++++--
 drivers/tty/cyclades.c                  |  8 +++++---
 drivers/tty/ehv_bytechan.c              |  2 ++
 drivers/tty/hvc/hvsi.c                  |  1 +
 drivers/tty/ipwireless/tty.c            |  1 +
 drivers/tty/isicom.c                    | 23 ++++++++++++----------
 drivers/tty/moxa.c                      |  4 ++++
 drivers/tty/mxser.c                     | 25 ++++++++++++++++--------
 drivers/tty/n_gsm.c                     | 11 +++++------
 drivers/tty/nozomi.c                    | 13 +++++++++----
 drivers/tty/pty.c                       |  2 +-
 drivers/tty/rocket.c                    |  2 ++
 drivers/tty/serial/68328serial.c        |  2 ++
 drivers/tty/serial/ifx6x60.c            |  5 ++++-
 drivers/tty/serial/kgdb_nmi.c           |  2 ++
 drivers/tty/serial/serial_core.c        |  6 ++++++
 drivers/tty/synclink.c                  |  1 +
 drivers/tty/synclink_gt.c               |  5 ++++-
 drivers/tty/synclinkmp.c                |  5 ++++-
 drivers/tty/tty_port.c                  | 18 +++++++++++++++--
 drivers/tty/vt/vt.c                     |  5 ++++-
 drivers/usb/gadget/u_serial.c           |  5 ++++-
 drivers/usb/serial/usb-serial.c         |  1 +
 include/linux/tty.h                     |  1 +
 net/irda/ircomm/ircomm_tty.c            |  1 +
 48 files changed, 202 insertions(+), 90 deletions(-)

-- 
1.8.0



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/9] TTY: isicom, stop using port->tty
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 2/9] TTY: pty, fix tty buffers leak Jiri Slaby
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

Do not access unsafe port->tty pointer when we have a safe tty
already. Use the safe one.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/isicom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/isicom.c b/drivers/tty/isicom.c
index d7492e1..5f3ecbc 100644
--- a/drivers/tty/isicom.c
+++ b/drivers/tty/isicom.c
@@ -603,7 +603,7 @@ static irqreturn_t isicom_interrupt(int irq, void *dev_id)
 			if (tty_port_cts_enabled(&port->port)) {
 				if (tty->hw_stopped) {
 					if (header & ISI_CTS) {
-						port->port.tty->hw_stopped = 0;
+						tty->hw_stopped = 0;
 						/* start tx ing */
 						port->status |= (ISI_TXOK
 							| ISI_CTS);
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/9] TTY: pty, fix tty buffers leak
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
  2012-11-15  8:49 ` [PATCH 1/9] TTY: isicom, stop using port->tty Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 3/9] ISDN: capi, use kref from tty_port Jiri Slaby
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue. So fix the drivers to fulfil this
assumption.

PTY is one of those, here we just need to use tty_port_put instead of
kfree. (Assuming tty_port_destructor does not need port->ops to be set
which we change here too.)

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/pty.c      | 2 +-
 drivers/tty/tty_port.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 0ce0b3e..a541ec8 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -408,7 +408,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
 static void pty_cleanup(struct tty_struct *tty)
 {
 	tty->port->itty = NULL;
-	kfree(tty->port);
+	tty_port_put(tty->port);
 }
 
 /* Traditional BSD devices */
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 416b42f..fdc42c2 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -128,7 +128,7 @@ static void tty_port_destructor(struct kref *kref)
 	if (port->xmit_buf)
 		free_page((unsigned long)port->xmit_buf);
 	tty_buffer_free_all(port);
-	if (port->ops->destruct)
+	if (port->ops && port->ops->destruct)
 		port->ops->destruct(port);
 	else
 		kfree(port);
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/9] ISDN: capi, use kref from tty_port
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
  2012-11-15  8:49 ` [PATCH 1/9] TTY: isicom, stop using port->tty Jiri Slaby
  2012-11-15  8:49 ` [PATCH 2/9] TTY: pty, fix tty buffers leak Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 4/9] MMC: sdio_uart, remove unused member from sdio_uart_port Jiri Slaby
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue. So fix the drivers to fulfil this
assumption.

Here it is enough to switch to refcounting in tty_port.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/isdn/capi/capi.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index c679867..89562a8 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -77,8 +77,6 @@ struct ackqueue_entry {
 };
 
 struct capiminor {
-	struct kref kref;
-
 	unsigned int      minor;
 
 	struct capi20_appl	*ap;
@@ -190,7 +188,20 @@ static void capiminor_del_all_ack(struct capiminor *mp)
 
 /* -------- struct capiminor ---------------------------------------- */
 
-static const struct tty_port_operations capiminor_port_ops; /* we have none */
+static void capiminor_destroy(struct tty_port *port)
+{
+	struct capiminor *mp = container_of(port, struct capiminor, port);
+
+	kfree_skb(mp->outskb);
+	skb_queue_purge(&mp->inqueue);
+	skb_queue_purge(&mp->outqueue);
+	capiminor_del_all_ack(mp);
+	kfree(mp);
+}
+
+static const struct tty_port_operations capiminor_port_ops = {
+	.destruct = capiminor_destroy,
+};
 
 static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 {
@@ -204,8 +215,6 @@ static struct capiminor *capiminor_alloc(struct capi20_appl *ap, u32 ncci)
 		return NULL;
 	}
 
-	kref_init(&mp->kref);
-
 	mp->ap = ap;
 	mp->ncci = ncci;
 	INIT_LIST_HEAD(&mp->ackqueue);
@@ -247,21 +256,10 @@ err_out2:
 	spin_unlock(&capiminors_lock);
 
 err_out1:
-	kfree(mp);
+	tty_port_put(&mp->port);
 	return NULL;
 }
 
-static void capiminor_destroy(struct kref *kref)
-{
-	struct capiminor *mp = container_of(kref, struct capiminor, kref);
-
-	kfree_skb(mp->outskb);
-	skb_queue_purge(&mp->inqueue);
-	skb_queue_purge(&mp->outqueue);
-	capiminor_del_all_ack(mp);
-	kfree(mp);
-}
-
 static struct capiminor *capiminor_get(unsigned int minor)
 {
 	struct capiminor *mp;
@@ -269,7 +267,7 @@ static struct capiminor *capiminor_get(unsigned int minor)
 	spin_lock(&capiminors_lock);
 	mp = capiminors[minor];
 	if (mp)
-		kref_get(&mp->kref);
+		tty_port_get(&mp->port);
 	spin_unlock(&capiminors_lock);
 
 	return mp;
@@ -277,7 +275,7 @@ static struct capiminor *capiminor_get(unsigned int minor)
 
 static inline void capiminor_put(struct capiminor *mp)
 {
-	kref_put(&mp->kref, capiminor_destroy);
+	tty_port_put(&mp->port);
 }
 
 static void capiminor_free(struct capiminor *mp)
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/9] MMC: sdio_uart, remove unused member from sdio_uart_port
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (2 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 3/9] ISDN: capi, use kref from tty_port Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 5/9] MMC: sdio, use kref from tty_port Jiri Slaby
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

tty from struct sdio_uart_port is unused. Proper refcounted tty in
tty_port->tty is used instead. So remove the member from that
structure.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/mmc/card/sdio_uart.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/mmc/card/sdio_uart.c b/drivers/mmc/card/sdio_uart.c
index d2339ea..369f7ba 100644
--- a/drivers/mmc/card/sdio_uart.c
+++ b/drivers/mmc/card/sdio_uart.c
@@ -67,7 +67,6 @@ struct uart_icount {
 struct sdio_uart_port {
 	struct tty_port		port;
 	struct kref		kref;
-	struct tty_struct	*tty;
 	unsigned int		index;
 	struct sdio_func	*func;
 	struct mutex		func_lock;
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/9] MMC: sdio, use kref from tty_port
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (3 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 4/9] MMC: sdio_uart, remove unused member from sdio_uart_port Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 6/9] TTY: n_gsm, " Jiri Slaby
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue. So fix the drivers to fulfil this
assumption.

Here it is enough to switch to refcounting in tty_port.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/mmc/card/sdio_uart.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/card/sdio_uart.c b/drivers/mmc/card/sdio_uart.c
index 369f7ba..bd57a11 100644
--- a/drivers/mmc/card/sdio_uart.c
+++ b/drivers/mmc/card/sdio_uart.c
@@ -66,7 +66,6 @@ struct uart_icount {
 
 struct sdio_uart_port {
 	struct tty_port		port;
-	struct kref		kref;
 	unsigned int		index;
 	struct sdio_func	*func;
 	struct mutex		func_lock;
@@ -92,7 +91,6 @@ static int sdio_uart_add_port(struct sdio_uart_port *port)
 {
 	int index, ret = -EBUSY;
 
-	kref_init(&port->kref);
 	mutex_init(&port->func_lock);
 	spin_lock_init(&port->write_lock);
 	if (kfifo_alloc(&port->xmit_fifo, FIFO_SIZE, GFP_KERNEL))
@@ -122,23 +120,15 @@ static struct sdio_uart_port *sdio_uart_port_get(unsigned index)
 	spin_lock(&sdio_uart_table_lock);
 	port = sdio_uart_table[index];
 	if (port)
-		kref_get(&port->kref);
+		tty_port_get(&port->port);
 	spin_unlock(&sdio_uart_table_lock);
 
 	return port;
 }
 
-static void sdio_uart_port_destroy(struct kref *kref)
-{
-	struct sdio_uart_port *port =
-		container_of(kref, struct sdio_uart_port, kref);
-	kfifo_free(&port->xmit_fifo);
-	kfree(port);
-}
-
 static void sdio_uart_port_put(struct sdio_uart_port *port)
 {
-	kref_put(&port->kref, sdio_uart_port_destroy);
+	tty_port_put(&port->port);
 }
 
 static void sdio_uart_port_remove(struct sdio_uart_port *port)
@@ -736,6 +726,14 @@ static void sdio_uart_shutdown(struct tty_port *tport)
 	sdio_uart_release_func(port);
 }
 
+static void sdio_uart_port_destroy(struct tty_port *tport)
+{
+	struct sdio_uart_port *port =
+		container_of(tport, struct sdio_uart_port, port);
+	kfifo_free(&port->xmit_fifo);
+	kfree(port);
+}
+
 /**
  *	sdio_uart_install	-	install method
  *	@driver: the driver in use (sdio_uart in our case)
@@ -1044,6 +1042,7 @@ static const struct tty_port_operations sdio_uart_port_ops = {
 	.carrier_raised = uart_carrier_raised,
 	.shutdown = sdio_uart_shutdown,
 	.activate = sdio_uart_activate,
+	.destruct = sdio_uart_port_destroy,
 };
 
 static const struct tty_operations sdio_uart_ops = {
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/9] TTY: n_gsm, use kref from tty_port
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (4 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 5/9] MMC: sdio, use kref from tty_port Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 7/9] TTY: introduce tty_port_destroy Jiri Slaby
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue. So fix the drivers to fulfil this
assumption.

Here it is enough to switch to refcounting in tty_port.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/n_gsm.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 1e8e8ce..dcc0430 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -134,7 +134,6 @@ struct gsm_dlci {
 #define DLCI_OPENING		1	/* Sending SABM not seen UA */
 #define DLCI_OPEN		2	/* SABM/UA complete */
 #define DLCI_CLOSING		3	/* Sending DISC not seen UA/DM */
-	struct kref ref;		/* freed from port or mux close */
 	struct mutex mutex;
 
 	/* Link layer */
@@ -1635,7 +1634,6 @@ static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
 	if (dlci == NULL)
 		return NULL;
 	spin_lock_init(&dlci->lock);
-	kref_init(&dlci->ref);
 	mutex_init(&dlci->mutex);
 	dlci->fifo = &dlci->_fifo;
 	if (kfifo_alloc(&dlci->_fifo, 4096, GFP_KERNEL) < 0) {
@@ -1669,9 +1667,9 @@ static struct gsm_dlci *gsm_dlci_alloc(struct gsm_mux *gsm, int addr)
  *
  *	Can sleep.
  */
-static void gsm_dlci_free(struct kref *ref)
+static void gsm_dlci_free(struct tty_port *port)
 {
-	struct gsm_dlci *dlci = container_of(ref, struct gsm_dlci, ref);
+	struct gsm_dlci *dlci = container_of(port, struct gsm_dlci, port);
 
 	del_timer_sync(&dlci->t1);
 	dlci->gsm->dlci[dlci->addr] = NULL;
@@ -1683,12 +1681,12 @@ static void gsm_dlci_free(struct kref *ref)
 
 static inline void dlci_get(struct gsm_dlci *dlci)
 {
-	kref_get(&dlci->ref);
+	tty_port_get(&dlci->port);
 }
 
 static inline void dlci_put(struct gsm_dlci *dlci)
 {
-	kref_put(&dlci->ref, gsm_dlci_free);
+	tty_port_put(&dlci->port);
 }
 
 /**
@@ -2874,6 +2872,7 @@ static void gsm_dtr_rts(struct tty_port *port, int onoff)
 static const struct tty_port_operations gsm_port_ops = {
 	.carrier_raised = gsm_carrier_raised,
 	.dtr_rts = gsm_dtr_rts,
+	.destruct = gsm_dlci_free,
 };
 
 static int gsmtty_install(struct tty_driver *driver, struct tty_struct *tty)
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/9] TTY: introduce tty_port_destroy
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (5 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 6/9] TTY: n_gsm, " Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 8/9] TTY: isicom, fix tty buffers memory leak Jiri Slaby
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue.

Those using refcounting are safe now, but for those which do not we
introduce a function to be called right before the tty_port is freed
by the drivers.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/tty_port.c | 16 +++++++++++++++-
 include/linux/tty.h    |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index fdc42c2..b7ff59d 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -122,12 +122,26 @@ void tty_port_free_xmit_buf(struct tty_port *port)
 }
 EXPORT_SYMBOL(tty_port_free_xmit_buf);
 
+/**
+ * tty_port_destroy -- destroy inited port
+ * @port: tty port to be doestroyed
+ *
+ * When a port was initialized using tty_port_init, one has to destroy the
+ * port by this function. Either indirectly by using tty_port refcounting
+ * (tty_port_put) or directly if refcounting is not used.
+ */
+void tty_port_destroy(struct tty_port *port)
+{
+	tty_buffer_free_all(port);
+}
+EXPORT_SYMBOL(tty_port_destroy);
+
 static void tty_port_destructor(struct kref *kref)
 {
 	struct tty_port *port = container_of(kref, struct tty_port, kref);
 	if (port->xmit_buf)
 		free_page((unsigned long)port->xmit_buf);
-	tty_buffer_free_all(port);
+	tty_port_destroy(port);
 	if (port->ops && port->ops->destruct)
 		port->ops->destruct(port);
 	else
diff --git a/include/linux/tty.h b/include/linux/tty.h
index d7ff88f..8db1b56 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -455,6 +455,7 @@ extern struct device *tty_port_register_device_attr(struct tty_port *port,
 		const struct attribute_group **attr_grp);
 extern int tty_port_alloc_xmit_buf(struct tty_port *port);
 extern void tty_port_free_xmit_buf(struct tty_port *port);
+extern void tty_port_destroy(struct tty_port *port);
 extern void tty_port_put(struct tty_port *port);
 
 static inline struct tty_port *tty_port_get(struct tty_port *port)
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/9] TTY: isicom, fix tty buffers memory leak
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (6 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 7/9] TTY: introduce tty_port_destroy Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-15  8:49 ` [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers Jiri Slaby
  2012-11-15 10:42 ` [PATCH 0/9] TTY: memory leaks patchset Alan Cox
  9 siblings, 0 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue. So fix the drivers to fulfil this
assumption.

This one is special as we need more work to be done. Previously,
the tty_port was initialized at module load time, but to be able to
destroy the port and init it again, we now do the initialization in
probe and destroy in remove. I.e. at more appropriate places for that.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/isicom.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/isicom.c b/drivers/tty/isicom.c
index 5f3ecbc..67f288d 100644
--- a/drivers/tty/isicom.c
+++ b/drivers/tty/isicom.c
@@ -1610,10 +1610,15 @@ static int __devinit isicom_probe(struct pci_dev *pdev,
 	if (retval < 0)
 		goto errunri;
 
-	for (index = 0; index < board->port_count; index++)
-		tty_port_register_device(&board->ports[index].port,
-				isicom_normal, board->index * 16 + index,
-				&pdev->dev);
+	for (index = 0; index < board->port_count; index++) {
+		struct tty_port *tport = &board->ports[index].port;
+		tty_port_init(tport);
+		tport->ops = &isicom_port_ops;
+		tport->close_delay = 50 * HZ/100;
+		tport->closing_wait = 3000 * HZ/100;
+		tty_port_register_device(tport, isicom_normal,
+				board->index * 16 + index, &pdev->dev);
+	}
 
 	return 0;
 
@@ -1635,8 +1640,10 @@ static void __devexit isicom_remove(struct pci_dev *pdev)
 	struct isi_board *board = pci_get_drvdata(pdev);
 	unsigned int i;
 
-	for (i = 0; i < board->port_count; i++)
+	for (i = 0; i < board->port_count; i++) {
 		tty_unregister_device(isicom_normal, board->index * 16 + i);
+		tty_port_destroy(&board->ports[i].port);
+	}
 
 	free_irq(board->irq, board);
 	pci_release_region(pdev, 3);
@@ -1655,13 +1662,9 @@ static int __init isicom_init(void)
 		isi_card[idx].ports = port;
 		spin_lock_init(&isi_card[idx].card_lock);
 		for (channel = 0; channel < 16; channel++, port++) {
-			tty_port_init(&port->port);
-			port->port.ops = &isicom_port_ops;
 			port->magic = ISICOM_MAGIC;
 			port->card = &isi_card[idx];
 			port->channel = channel;
-			port->port.close_delay = 50 * HZ/100;
-			port->port.closing_wait = 3000 * HZ/100;
 			port->status = 0;
 			/*  . . .  */
 		}
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (7 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 8/9] TTY: isicom, fix tty buffers memory leak Jiri Slaby
@ 2012-11-15  8:49 ` Jiri Slaby
  2012-11-18 13:39   ` Tilman Schmidt
  2012-11-27 16:52   ` Peter Hurley
  2012-11-15 10:42 ` [PATCH 0/9] TTY: memory leaks patchset Alan Cox
  9 siblings, 2 replies; 20+ messages in thread
From: Jiri Slaby @ 2012-11-15  8:49 UTC (permalink / raw)
  To: gregkh; +Cc: alan, linux-kernel, jirislaby

After commit "TTY: move tty buffers to tty_port", the tty buffers are
not freed in some drivers. This is because tty_port_destructor is not
called whenever a tty_port is freed. This was an assumption I counted
with but was unfortunately untrue. So fix the drivers to fulfil this
assumption.

To be sure, the TTY buffers (and later some stuff) are gone along with
the tty_port, we have to call tty_port_destroy at tear-down places.
This is mostly where the structure containing a tty_port is freed.
This patch does exactly that -- put tty_port_destroy at those places.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 arch/alpha/kernel/srmcons.c             |  5 ++++-
 arch/ia64/hp/sim/simserial.c            |  1 +
 arch/m68k/emu/nfcon.c                   |  6 ++++--
 arch/parisc/kernel/pdc_cons.c           |  5 +++--
 arch/um/drivers/line.c                  |  2 ++
 arch/xtensa/platforms/iss/console.c     |  1 +
 drivers/char/pcmcia/synclink_cs.c       |  5 ++++-
 drivers/char/ttyprintk.c                |  4 +++-
 drivers/isdn/gigaset/common.c           | 10 ++++++----
 drivers/isdn/i4l/isdn_tty.c             |  4 ++++
 drivers/misc/pti.c                      |  7 +++++--
 drivers/net/usb/hso.c                   |  5 +++--
 drivers/s390/char/con3215.c             |  1 +
 drivers/s390/char/sclp_tty.c            |  4 +++-
 drivers/s390/char/sclp_vt220.c          |  2 ++
 drivers/s390/char/tty3270.c             |  2 ++
 drivers/staging/ccg/u_serial.c          |  5 ++++-
 drivers/staging/dgrp/dgrp_specproc.c    |  2 ++
 drivers/staging/dgrp/dgrp_tty.c         |  4 +++-
 drivers/staging/ipack/devices/ipoctal.c |  2 ++
 drivers/tty/amiserial.c                 |  2 ++
 drivers/tty/bfin_jtag_comm.c            |  6 ++++--
 drivers/tty/cyclades.c                  |  8 +++++---
 drivers/tty/ehv_bytechan.c              |  2 ++
 drivers/tty/hvc/hvsi.c                  |  1 +
 drivers/tty/ipwireless/tty.c            |  1 +
 drivers/tty/moxa.c                      |  4 ++++
 drivers/tty/mxser.c                     | 25 +++++++++++++++++--------
 drivers/tty/nozomi.c                    | 13 +++++++++----
 drivers/tty/rocket.c                    |  2 ++
 drivers/tty/serial/68328serial.c        |  2 ++
 drivers/tty/serial/ifx6x60.c            |  5 ++++-
 drivers/tty/serial/kgdb_nmi.c           |  2 ++
 drivers/tty/serial/serial_core.c        |  6 ++++++
 drivers/tty/synclink.c                  |  1 +
 drivers/tty/synclink_gt.c               |  5 ++++-
 drivers/tty/synclinkmp.c                |  5 ++++-
 drivers/tty/vt/vt.c                     |  5 ++++-
 drivers/usb/gadget/u_serial.c           |  5 ++++-
 drivers/usb/serial/usb-serial.c         |  1 +
 net/irda/ircomm/ircomm_tty.c            |  1 +
 41 files changed, 139 insertions(+), 40 deletions(-)

diff --git a/arch/alpha/kernel/srmcons.c b/arch/alpha/kernel/srmcons.c
index 5d58652..59b7bba 100644
--- a/arch/alpha/kernel/srmcons.c
+++ b/arch/alpha/kernel/srmcons.c
@@ -205,7 +205,6 @@ static const struct tty_operations srmcons_ops = {
 static int __init
 srmcons_init(void)
 {
-	tty_port_init(&srmcons_singleton.port);
 	setup_timer(&srmcons_singleton.timer, srmcons_receive_chars,
 			(unsigned long)&srmcons_singleton);
 	if (srm_is_registered_console) {
@@ -215,6 +214,9 @@ srmcons_init(void)
 		driver = alloc_tty_driver(MAX_SRM_CONSOLE_DEVICES);
 		if (!driver)
 			return -ENOMEM;
+
+		tty_port_init(&srmcons_singleton.port);
+
 		driver->driver_name = "srm";
 		driver->name = "srm";
 		driver->major = 0; 	/* dynamic */
@@ -227,6 +229,7 @@ srmcons_init(void)
 		err = tty_register_driver(driver);
 		if (err) {
 			put_tty_driver(driver);
+			tty_port_destroy(&srmcons_singleton.port);
 			return err;
 		}
 		srmcons_driver = driver;
diff --git a/arch/ia64/hp/sim/simserial.c b/arch/ia64/hp/sim/simserial.c
index ec536e4..fc3924d 100644
--- a/arch/ia64/hp/sim/simserial.c
+++ b/arch/ia64/hp/sim/simserial.c
@@ -555,6 +555,7 @@ static int __init simrs_init(void)
 	return 0;
 err_free_tty:
 	put_tty_driver(hp_simserial_driver);
+	tty_port_destroy(&state->port);
 	return retval;
 }
 
diff --git a/arch/m68k/emu/nfcon.c b/arch/m68k/emu/nfcon.c
index 16d170f..6685bf4 100644
--- a/arch/m68k/emu/nfcon.c
+++ b/arch/m68k/emu/nfcon.c
@@ -120,8 +120,6 @@ static int __init nfcon_init(void)
 {
 	int res;
 
-	tty_port_init(&nfcon_tty_port);
-
 	stderr_id = nf_get_id("NF_STDERR");
 	if (!stderr_id)
 		return -ENODEV;
@@ -130,6 +128,8 @@ static int __init nfcon_init(void)
 	if (!nfcon_tty_driver)
 		return -ENOMEM;
 
+	tty_port_init(&nfcon_tty_port);
+
 	nfcon_tty_driver->driver_name = "nfcon";
 	nfcon_tty_driver->name = "nfcon";
 	nfcon_tty_driver->type = TTY_DRIVER_TYPE_SYSTEM;
@@ -143,6 +143,7 @@ static int __init nfcon_init(void)
 	if (res) {
 		pr_err("failed to register nfcon tty driver\n");
 		put_tty_driver(nfcon_tty_driver);
+		tty_port_destroy(&nfcon_tty_port);
 		return res;
 	}
 
@@ -157,6 +158,7 @@ static void __exit nfcon_exit(void)
 	unregister_console(&nf_console);
 	tty_unregister_driver(nfcon_tty_driver);
 	put_tty_driver(nfcon_tty_driver);
+	tty_port_destroy(&nfcon_tty_port);
 }
 
 module_init(nfcon_init);
diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c
index 8823863..efc5e7d 100644
--- a/arch/parisc/kernel/pdc_cons.c
+++ b/arch/parisc/kernel/pdc_cons.c
@@ -186,13 +186,13 @@ static int __init pdc_console_tty_driver_init(void)
 	printk(KERN_INFO "The PDC console driver is still registered, removing CON_BOOT flag\n");
 	pdc_cons.flags &= ~CON_BOOT;
 
-	tty_port_init(&tty_port);
-
 	pdc_console_tty_driver = alloc_tty_driver(1);
 
 	if (!pdc_console_tty_driver)
 		return -ENOMEM;
 
+	tty_port_init(&tty_port);
+
 	pdc_console_tty_driver->driver_name = "pdc_cons";
 	pdc_console_tty_driver->name = "ttyB";
 	pdc_console_tty_driver->major = MUX_MAJOR;
@@ -207,6 +207,7 @@ static int __init pdc_console_tty_driver_init(void)
 	err = tty_register_driver(pdc_console_tty_driver);
 	if (err) {
 		printk(KERN_ERR "Unable to register the PDC console TTY driver\n");
+		tty_port_destroy(&tty_port);
 		return err;
 	}
 
diff --git a/arch/um/drivers/line.c b/arch/um/drivers/line.c
index fd9a15b..9ffc28b 100644
--- a/arch/um/drivers/line.c
+++ b/arch/um/drivers/line.c
@@ -584,6 +584,8 @@ int register_lines(struct line_driver *line_driver,
 		printk(KERN_ERR "register_lines : can't register %s driver\n",
 		       line_driver->name);
 		put_tty_driver(driver);
+		for (i = 0; i < nlines; i++)
+			tty_port_destroy(&lines[i].port);
 		return err;
 	}
 
diff --git a/arch/xtensa/platforms/iss/console.c b/arch/xtensa/platforms/iss/console.c
index 7e74895..8207a11 100644
--- a/arch/xtensa/platforms/iss/console.c
+++ b/arch/xtensa/platforms/iss/console.c
@@ -221,6 +221,7 @@ static __exit void rs_exit(void)
 		printk("ISS_SERIAL: failed to unregister serial driver (%d)\n",
 		       error);
 	put_tty_driver(serial_driver);
+	tty_port_destroy(&serial_port);
 }
 
 
diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 21721d2..b66eaa0 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -549,8 +549,10 @@ static int mgslpc_probe(struct pcmcia_device *link)
     /* Initialize the struct pcmcia_device structure */
 
     ret = mgslpc_config(link);
-    if (ret)
+    if (ret) {
+	    tty_port_destroy(&info->port);
 	    return ret;
+    }
 
     mgslpc_add_device(info);
 
@@ -2757,6 +2759,7 @@ static void mgslpc_remove_device(MGSLPC_INFO *remove_info)
 			hdlcdev_exit(info);
 #endif
 			release_resources(info);
+			tty_port_destroy(&info->port);
 			kfree(info);
 			mgslpc_device_count--;
 			return;
diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c
index af98f6d..4945bd3 100644
--- a/drivers/char/ttyprintk.c
+++ b/drivers/char/ttyprintk.c
@@ -179,7 +179,6 @@ static int __init ttyprintk_init(void)
 {
 	int ret = -ENOMEM;
 
-	tty_port_init(&tpk_port.port);
 	tpk_port.port.ops = &null_ops;
 	mutex_init(&tpk_port.port_write_mutex);
 
@@ -190,6 +189,8 @@ static int __init ttyprintk_init(void)
 	if (IS_ERR(ttyprintk_driver))
 		return PTR_ERR(ttyprintk_driver);
 
+	tty_port_init(&tpk_port.port);
+
 	ttyprintk_driver->driver_name = "ttyprintk";
 	ttyprintk_driver->name = "ttyprintk";
 	ttyprintk_driver->major = TTYAUX_MAJOR;
@@ -211,6 +212,7 @@ static int __init ttyprintk_init(void)
 error:
 	tty_unregister_driver(ttyprintk_driver);
 	put_tty_driver(ttyprintk_driver);
+	tty_port_destroy(&tpk_port.port);
 	ttyprintk_driver = NULL;
 	return ret;
 }
diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
index 30a6b17..bc9d89a 100644
--- a/drivers/isdn/gigaset/common.c
+++ b/drivers/isdn/gigaset/common.c
@@ -518,6 +518,7 @@ f_bcs:	gig_dbg(DEBUG_INIT, "freeing bcs[]");
 	kfree(cs->bcs);
 f_cs:	gig_dbg(DEBUG_INIT, "freeing cs");
 	mutex_unlock(&cs->mutex);
+	tty_port_destroy(&cs->port);
 	free_cs(cs);
 }
 EXPORT_SYMBOL_GPL(gigaset_freecs);
@@ -751,14 +752,14 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 	gig_dbg(DEBUG_INIT, "setting up iif");
 	if (gigaset_isdn_regdev(cs, modulename) < 0) {
 		pr_err("error registering ISDN device\n");
-		goto error;
+		goto error_port;
 	}
 
 	make_valid(cs, VALID_ID);
 	++cs->cs_init;
 	gig_dbg(DEBUG_INIT, "setting up hw");
 	if (cs->ops->initcshw(cs) < 0)
-		goto error;
+		goto error_port;
 
 	++cs->cs_init;
 
@@ -773,7 +774,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 		gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
 		if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
 			pr_err("could not allocate channel %d data\n", i);
-			goto error;
+			goto error_port;
 		}
 	}
 
@@ -786,7 +787,8 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
 
 	gig_dbg(DEBUG_INIT, "cs initialized");
 	return cs;
-
+error_port:
+	tty_port_destroy(&cs->port);
 error:
 	gig_dbg(DEBUG_INIT, "failed");
 	gigaset_freecs(cs);
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index b817809..e09dc8a 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1849,6 +1849,8 @@ err_unregister:
 		kfree(info->fax);
 #endif
 		kfree(info->port.xmit_buf - 4);
+		info->port.xmit_buf = NULL;
+		tty_port_destroy(&info->port);
 	}
 	tty_unregister_driver(m->tty_modem);
 err:
@@ -1870,6 +1872,8 @@ isdn_tty_exit(void)
 		kfree(info->fax);
 #endif
 		kfree(info->port.xmit_buf - 4);
+		info->port.xmit_buf = NULL;
+		tty_port_destroy(&info->port);
 	}
 	tty_unregister_driver(dev->mdm.tty_modem);
 	put_tty_driver(dev->mdm.tty_modem);
diff --git a/drivers/misc/pti.c b/drivers/misc/pti.c
index 4999b34..a1f0d17 100644
--- a/drivers/misc/pti.c
+++ b/drivers/misc/pti.c
@@ -882,11 +882,14 @@ err:
 static void __devexit pti_pci_remove(struct pci_dev *pdev)
 {
 	struct pti_dev *drv_data = pci_get_drvdata(pdev);
+	unsigned int a;
 
 	unregister_console(&pti_console);
 
-	tty_unregister_device(pti_tty_driver, 0);
-	tty_unregister_device(pti_tty_driver, 1);
+	for (a = 0; a < PTITTY_MINOR_NUM; a++) {
+		tty_unregister_device(pti_tty_driver, a);
+		tty_port_destroy(&drv_data->port[a]);
+	}
 
 	iounmap(drv_data->pti_ioaddr);
 	pci_set_drvdata(pdev, NULL);
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 605a4ba..cd8ccb2 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2274,6 +2274,7 @@ static void hso_serial_common_free(struct hso_serial *serial)
 	/* unlink and free TX URB */
 	usb_free_urb(serial->tx_urb);
 	kfree(serial->tx_data);
+	tty_port_destroy(&serial->port);
 }
 
 static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
@@ -2283,12 +2284,12 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
 	int minor;
 	int i;
 
+	tty_port_init(&serial->port);
+
 	minor = get_free_serial_index();
 	if (minor < 0)
 		goto exit;
 
-	tty_port_init(&serial->port);
-
 	/* register our minor number */
 	serial->parent->dev = tty_port_register_device(&serial->port, tty_drv,
 			minor, &serial->parent->interface->dev);
diff --git a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
index 9ffb6d5..8fb014f 100644
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -677,6 +677,7 @@ static void raw3215_free_info(struct raw3215_info *raw)
 {
 	kfree(raw->inbuf);
 	kfree(raw->buffer);
+	tty_port_destroy(&raw->port);
 	kfree(raw);
 }
 
diff --git a/drivers/s390/char/sclp_tty.c b/drivers/s390/char/sclp_tty.c
index 30ec09e..877fbc3 100644
--- a/drivers/s390/char/sclp_tty.c
+++ b/drivers/s390/char/sclp_tty.c
@@ -547,7 +547,6 @@ sclp_tty_init(void)
 		sclp_tty_tolower = 1;
 	}
 	sclp_tty_chars_count = 0;
-	tty_port_init(&sclp_port);
 
 	rc = sclp_register(&sclp_input_event);
 	if (rc) {
@@ -555,6 +554,8 @@ sclp_tty_init(void)
 		return rc;
 	}
 
+	tty_port_init(&sclp_port);
+
 	driver->driver_name = "sclp_line";
 	driver->name = "sclp_line";
 	driver->major = TTY_MAJOR;
@@ -571,6 +572,7 @@ sclp_tty_init(void)
 	rc = tty_register_driver(driver);
 	if (rc) {
 		put_tty_driver(driver);
+		tty_port_destroy(&sclp_port);
 		return rc;
 	}
 	sclp_tty_driver = driver;
diff --git a/drivers/s390/char/sclp_vt220.c b/drivers/s390/char/sclp_vt220.c
index 7e60f3d..effcc87 100644
--- a/drivers/s390/char/sclp_vt220.c
+++ b/drivers/s390/char/sclp_vt220.c
@@ -615,6 +615,7 @@ static void __init __sclp_vt220_cleanup(void)
 		return;
 	sclp_unregister(&sclp_vt220_register);
 	__sclp_vt220_free_pages();
+	tty_port_destroy(&sclp_vt220_port);
 }
 
 /* Allocate buffer pages and register with sclp core. Controlled by init
@@ -650,6 +651,7 @@ out:
 	if (rc) {
 		__sclp_vt220_free_pages();
 		sclp_vt220_init_count--;
+		tty_port_destroy(&sclp_vt220_port);
 	}
 	return rc;
 }
diff --git a/drivers/s390/char/tty3270.c b/drivers/s390/char/tty3270.c
index 482ee02..43ea059 100644
--- a/drivers/s390/char/tty3270.c
+++ b/drivers/s390/char/tty3270.c
@@ -722,6 +722,7 @@ out_pages:
 	while (pages--)
 		free_pages((unsigned long) tp->freemem_pages[pages], 0);
 	kfree(tp->freemem_pages);
+	tty_port_destroy(&tp->port);
 out_tp:
 	kfree(tp);
 out_err:
@@ -744,6 +745,7 @@ tty3270_free_view(struct tty3270 *tp)
 	for (pages = 0; pages < TTY3270_STRING_PAGES; pages++)
 		free_pages((unsigned long) tp->freemem_pages[pages], 0);
 	kfree(tp->freemem_pages);
+	tty_port_destroy(&tp->port);
 	kfree(tp);
 }
 
diff --git a/drivers/staging/ccg/u_serial.c b/drivers/staging/ccg/u_serial.c
index 5b3f5ff..373c406 100644
--- a/drivers/staging/ccg/u_serial.c
+++ b/drivers/staging/ccg/u_serial.c
@@ -1140,8 +1140,10 @@ int gserial_setup(struct usb_gadget *g, unsigned count)
 
 	return status;
 fail:
-	while (count--)
+	while (count--) {
+		tty_port_destroy(&ports[count].port->port);
 		kfree(ports[count].port);
+	}
 	put_tty_driver(gs_tty_driver);
 	gs_tty_driver = NULL;
 	return status;
@@ -1195,6 +1197,7 @@ void gserial_cleanup(void)
 
 		WARN_ON(port->port_usb != NULL);
 
+		tty_port_destroy(&port->port);
 		kfree(port);
 	}
 	n_ports = 0;
diff --git a/drivers/staging/dgrp/dgrp_specproc.c b/drivers/staging/dgrp/dgrp_specproc.c
index db91f67..c214078 100644
--- a/drivers/staging/dgrp/dgrp_specproc.c
+++ b/drivers/staging/dgrp/dgrp_specproc.c
@@ -752,6 +752,8 @@ static int dgrp_add_id(long id)
 
 	return 0;
 
+	/* FIXME this guy should free the tty driver stored in nd and destroy
+	 * all channel ports */
 error_out:
 	kfree(nd);
 	return ret;
diff --git a/drivers/staging/dgrp/dgrp_tty.c b/drivers/staging/dgrp/dgrp_tty.c
index e125b03..0db4c05 100644
--- a/drivers/staging/dgrp/dgrp_tty.c
+++ b/drivers/staging/dgrp/dgrp_tty.c
@@ -3119,6 +3119,7 @@ static void dgrp_tty_hangup(struct tty_struct *tty)
 void
 dgrp_tty_uninit(struct nd_struct *nd)
 {
+	unsigned int i;
 	char id[3];
 
 	ID_TO_CHAR(nd->nd_ID, id);
@@ -3152,6 +3153,8 @@ dgrp_tty_uninit(struct nd_struct *nd)
 		put_tty_driver(nd->nd_xprint_ttdriver);
 		nd->nd_ttdriver_flags &= ~XPRINT_TTDRV_REG;
 	}
+	for (i = 0; i < CHAN_MAX; i++)
+		tty_port_destroy(&nd->nd_chan[i].port);
 }
 
 
@@ -3335,7 +3338,6 @@ dgrp_tty_init(struct nd_struct *nd)
 		init_waitqueue_head(&(ch->ch_pun.un_open_wait));
 		init_waitqueue_head(&(ch->ch_pun.un_close_wait));
 		tty_port_init(&ch->port);
-		tty_port_init(&ch->port);
 	}
 	return 0;
 }
diff --git a/drivers/staging/ipack/devices/ipoctal.c b/drivers/staging/ipack/devices/ipoctal.c
index b6a72e6..cd41209 100644
--- a/drivers/staging/ipack/devices/ipoctal.c
+++ b/drivers/staging/ipack/devices/ipoctal.c
@@ -414,6 +414,7 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, unsigned int bus_nr,
 		tty_dev = tty_port_register_device(&channel->tty_port, tty, i, NULL);
 		if (IS_ERR(tty_dev)) {
 			dev_err(&ipoctal->dev->dev, "Failed to register tty device.\n");
+			tty_port_destroy(&channel->tty_port);
 			continue;
 		}
 		dev_set_drvdata(tty_dev, channel);
@@ -699,6 +700,7 @@ static void __ipoctal_remove(struct ipoctal *ipoctal)
 		struct ipoctal_channel *channel = &ipoctal->channel[i];
 		tty_unregister_device(ipoctal->tty_drv, i);
 		tty_port_free_xmit_buf(&channel->tty_port);
+		tty_port_destroy(&channel->tty_port);
 	}
 
 	tty_unregister_driver(ipoctal->tty_drv);
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 42d0a25..9d7d00c 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -1771,6 +1771,7 @@ fail_free_irq:
 fail_unregister:
 	tty_unregister_driver(serial_driver);
 fail_put_tty_driver:
+	tty_port_destroy(&state->tport);
 	put_tty_driver(serial_driver);
 	return error;
 }
@@ -1785,6 +1786,7 @@ static int __exit amiga_serial_remove(struct platform_device *pdev)
 		printk("SERIAL: failed to unregister serial driver (%d)\n",
 		       error);
 	put_tty_driver(serial_driver);
+	tty_port_destroy(&state->tport);
 
 	free_irq(IRQ_AMIGA_TBE, state);
 	free_irq(IRQ_AMIGA_RBF, state);
diff --git a/drivers/tty/bfin_jtag_comm.c b/drivers/tty/bfin_jtag_comm.c
index 02b7d3a..1cfcdbf 100644
--- a/drivers/tty/bfin_jtag_comm.c
+++ b/drivers/tty/bfin_jtag_comm.c
@@ -240,8 +240,6 @@ static int __init bfin_jc_init(void)
 {
 	int ret;
 
-	tty_port_init(&port);
-
 	bfin_jc_kthread = kthread_create(bfin_jc_emudat_manager, NULL, DRV_NAME);
 	if (IS_ERR(bfin_jc_kthread))
 		return PTR_ERR(bfin_jc_kthread);
@@ -257,6 +255,8 @@ static int __init bfin_jc_init(void)
 	if (!bfin_jc_driver)
 		goto err_driver;
 
+	tty_port_init(&port);
+
 	bfin_jc_driver->driver_name  = DRV_NAME;
 	bfin_jc_driver->name         = DEV_NAME;
 	bfin_jc_driver->type         = TTY_DRIVER_TYPE_SERIAL;
@@ -274,6 +274,7 @@ static int __init bfin_jc_init(void)
 	return 0;
 
  err:
+	tty_port_destroy(&port);
 	put_tty_driver(bfin_jc_driver);
  err_driver:
 	kfree(bfin_jc_write_buf.buf);
@@ -289,6 +290,7 @@ static void __exit bfin_jc_exit(void)
 	kfree(bfin_jc_write_buf.buf);
 	tty_unregister_driver(bfin_jc_driver);
 	put_tty_driver(bfin_jc_driver);
+	tty_port_destroy(&port);
 }
 module_exit(bfin_jc_exit);
 
diff --git a/drivers/tty/cyclades.c b/drivers/tty/cyclades.c
index 0a6a0bc..de25774 100644
--- a/drivers/tty/cyclades.c
+++ b/drivers/tty/cyclades.c
@@ -3934,7 +3934,7 @@ err:
 static void __devexit cy_pci_remove(struct pci_dev *pdev)
 {
 	struct cyclades_card *cinfo = pci_get_drvdata(pdev);
-	unsigned int i;
+	unsigned int i, channel;
 
 	/* non-Z with old PLX */
 	if (!cy_is_Z(cinfo) && (readb(cinfo->base_addr + CyPLX_VER) & 0x0f) ==
@@ -3960,9 +3960,11 @@ static void __devexit cy_pci_remove(struct pci_dev *pdev)
 	pci_release_regions(pdev);
 
 	cinfo->base_addr = NULL;
-	for (i = cinfo->first_line; i < cinfo->first_line +
-			cinfo->nports; i++)
+	for (channel = 0, i = cinfo->first_line; i < cinfo->first_line +
+			cinfo->nports; i++, channel++) {
 		tty_unregister_device(cy_serial_driver, i);
+		tty_port_destroy(&cinfo->ports[channel].port);
+	}
 	cinfo->nports = 0;
 	kfree(cinfo->ports);
 }
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index 4ab936b..4193afb 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -757,6 +757,7 @@ static int __devinit ehv_bc_tty_probe(struct platform_device *pdev)
 	return 0;
 
 error:
+	tty_port_destroy(&bc->port);
 	irq_dispose_mapping(bc->tx_irq);
 	irq_dispose_mapping(bc->rx_irq);
 
@@ -770,6 +771,7 @@ static int ehv_bc_tty_remove(struct platform_device *pdev)
 
 	tty_unregister_device(ehv_bc_driver, bc - bcs);
 
+	tty_port_destroy(&bc->port);
 	irq_dispose_mapping(bc->tx_irq);
 	irq_dispose_mapping(bc->rx_irq);
 
diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index 5b95b4f..68357a6 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1218,6 +1218,7 @@ static int __init hvsi_console_init(void)
 		if (hp->virq == 0) {
 			printk(KERN_ERR "%s: couldn't create irq mapping for 0x%x\n",
 				__func__, irq[0]);
+			tty_port_destroy(&hp->port);
 			continue;
 		}
 
diff --git a/drivers/tty/ipwireless/tty.c b/drivers/tty/ipwireless/tty.c
index 160f0ad..2cde13d 100644
--- a/drivers/tty/ipwireless/tty.c
+++ b/drivers/tty/ipwireless/tty.c
@@ -566,6 +566,7 @@ void ipwireless_tty_free(struct ipw_tty *tty)
 			ipwireless_disassociate_network_ttys(network,
 							     ttyj->channel_idx);
 			tty_unregister_device(ipw_tty_driver, j);
+			tty_port_destroy(&ttyj->port);
 			ttys[j] = NULL;
 			mutex_unlock(&ttyj->ipw_tty_mutex);
 			kfree(ttyj);
diff --git a/drivers/tty/moxa.c b/drivers/tty/moxa.c
index 56e616b..da112e1 100644
--- a/drivers/tty/moxa.c
+++ b/drivers/tty/moxa.c
@@ -895,6 +895,8 @@ static int moxa_init_board(struct moxa_board_conf *brd, struct device *dev)
 
 	return 0;
 err_free:
+	for (i = 0; i < MAX_PORTS_PER_BOARD; i++)
+		tty_port_destroy(&brd->ports[i].port);
 	kfree(brd->ports);
 err:
 	return ret;
@@ -919,6 +921,8 @@ static void moxa_board_deinit(struct moxa_board_conf *brd)
 				tty_kref_put(tty);
 			}
 		}
+	for (a = 0; a < MAX_PORTS_PER_BOARD; a++)
+		tty_port_destroy(&brd->ports[a].port);
 	while (1) {
 		opened = 0;
 		for (a = 0; a < brd->numPorts; a++)
diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index cfda47d..802a248 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -2411,14 +2411,27 @@ static int __devinit mxser_initbrd(struct mxser_board *brd,
 
 	retval = request_irq(brd->irq, mxser_interrupt, IRQF_SHARED, "mxser",
 			brd);
-	if (retval)
+	if (retval) {
+		for (i = 0; i < brd->info->nports; i++)
+			tty_port_destroy(&brd->ports[i].port);
 		printk(KERN_ERR "Board %s: Request irq failed, IRQ (%d) may "
 			"conflict with another device.\n",
 			brd->info->name, brd->irq);
+	}
 
 	return retval;
 }
 
+static void mxser_board_remove(struct mxser_board *brd)
+{
+	unsigned int i;
+
+	for (i = 0; i < brd->info->nports; i++) {
+		tty_unregister_device(mxvar_sdriver, brd->idx + i);
+		tty_port_destroy(&brd->ports[i].port);
+	}
+}
+
 static int __init mxser_get_ISA_conf(int cap, struct mxser_board *brd)
 {
 	int id, i, bits, ret;
@@ -2649,10 +2662,8 @@ static void __devexit mxser_remove(struct pci_dev *pdev)
 {
 #ifdef CONFIG_PCI
 	struct mxser_board *brd = pci_get_drvdata(pdev);
-	unsigned int i;
 
-	for (i = 0; i < brd->info->nports; i++)
-		tty_unregister_device(mxvar_sdriver, brd->idx + i);
+	mxser_board_remove(brd);
 
 	free_irq(pdev->irq, brd);
 	pci_release_region(pdev, 2);
@@ -2748,15 +2759,13 @@ err_put:
 
 static void __exit mxser_module_exit(void)
 {
-	unsigned int i, j;
+	unsigned int i;
 
 	pci_unregister_driver(&mxser_driver);
 
 	for (i = 0; i < MXSER_BOARDS; i++) /* ISA remains */
 		if (mxser_boards[i].info != NULL)
-			for (j = 0; j < mxser_boards[i].info->nports; j++)
-				tty_unregister_device(mxvar_sdriver,
-						mxser_boards[i].idx + j);
+			mxser_board_remove(&mxser_boards[i]);
 	tty_unregister_driver(mxvar_sdriver);
 	put_tty_driver(mxvar_sdriver);
 
diff --git a/drivers/tty/nozomi.c b/drivers/tty/nozomi.c
index b917c94..cb764d2 100644
--- a/drivers/tty/nozomi.c
+++ b/drivers/tty/nozomi.c
@@ -1479,6 +1479,7 @@ static int __devinit nozomi_card_init(struct pci_dev *pdev,
 		if (IS_ERR(tty_dev)) {
 			ret = PTR_ERR(tty_dev);
 			dev_err(&pdev->dev, "Could not allocate tty?\n");
+			tty_port_destroy(&port->port);
 			goto err_free_tty;
 		}
 	}
@@ -1486,8 +1487,10 @@ static int __devinit nozomi_card_init(struct pci_dev *pdev,
 	return 0;
 
 err_free_tty:
-	for (i = dc->index_start; i < dc->index_start + MAX_PORT; ++i)
-		tty_unregister_device(ntty_driver, i);
+	for (i = 0; i < MAX_PORT; ++i) {
+		tty_unregister_device(ntty_driver, dc->index_start + i);
+		tty_port_destroy(&dc->port[i].port);
+	}
 err_free_kfifo:
 	for (i = 0; i < MAX_PORT; i++)
 		kfifo_free(&dc->port[i].fifo_ul);
@@ -1520,8 +1523,10 @@ static void __devexit tty_exit(struct nozomi *dc)
 	   complete off a hangup method ? */
 	while (dc->open_ttys)
 		msleep(1);
-	for (i = dc->index_start; i < dc->index_start + MAX_PORT; ++i)
-		tty_unregister_device(ntty_driver, i);
+	for (i = 0; i < MAX_PORT; ++i) {
+		tty_unregister_device(ntty_driver, dc->index_start + i);
+		tty_port_destroy(&dc->port[i].port);
+	}
 }
 
 /* Deallocate memory for one device */
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index 9700d34..d9056da 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -673,6 +673,7 @@ static void init_r_port(int board, int aiop, int chan, struct pci_dev *pci_dev)
 	if (sInitChan(ctlp, &info->channel, aiop, chan) == 0) {
 		printk(KERN_ERR "RocketPort sInitChan(%d, %d, %d) failed!\n",
 				board, aiop, chan);
+		tty_port_destroy(&info->port);
 		kfree(info);
 		return;
 	}
@@ -2357,6 +2358,7 @@ static void rp_cleanup_module(void)
 	for (i = 0; i < MAX_RP_PORTS; i++)
 		if (rp_table[i]) {
 			tty_unregister_device(rocket_driver, i);
+			tty_port_destroy(&rp_table[i]->port);
 			kfree(rp_table[i]);
 		}
 
diff --git a/drivers/tty/serial/68328serial.c b/drivers/tty/serial/68328serial.c
index 66c38a3..f99a845 100644
--- a/drivers/tty/serial/68328serial.c
+++ b/drivers/tty/serial/68328serial.c
@@ -1225,6 +1225,8 @@ rs68328_init(void)
 
 	if (tty_register_driver(serial_driver)) {
 		put_tty_driver(serial_driver);
+		for (i = 0; i < NR_PORTS; i++)
+			tty_port_destroy(&m68k_soft[i].tport);
 		printk(KERN_ERR "Couldn't register serial driver\n");
 		return -ENOMEM;
 	}
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index fbda374..b97e348 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -822,6 +822,7 @@ static void ifx_spi_free_port(struct ifx_spi_device *ifx_dev)
 {
 	if (ifx_dev->tty_dev)
 		tty_unregister_device(tty_drv, ifx_dev->minor);
+	tty_port_destroy(&ifx_dev->tty_port);
 	kfifo_free(&ifx_dev->tx_fifo);
 }
 
@@ -855,10 +856,12 @@ static int ifx_spi_create_port(struct ifx_spi_device *ifx_dev)
 		dev_dbg(&ifx_dev->spi_dev->dev,
 			"%s: registering tty device failed", __func__);
 		ret = PTR_ERR(ifx_dev->tty_dev);
-		goto error_ret;
+		goto error_port;
 	}
 	return 0;
 
+error_port:
+	tty_port_destroy(pport);
 error_ret:
 	ifx_spi_free_port(ifx_dev);
 	return ret;
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
index d185247..6ac2b79 100644
--- a/drivers/tty/serial/kgdb_nmi.c
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -266,6 +266,7 @@ static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
 	}
 	return 0;
 err:
+	tty_port_destroy(&priv->port);
 	kfree(priv);
 	return ret;
 }
@@ -275,6 +276,7 @@ static void kgdb_nmi_tty_cleanup(struct tty_struct *tty)
 	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
 
 	tty->driver_data = NULL;
+	tty_port_destroy(&priv->port);
 	kfree(priv);
 }
 
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 44274ea..2c7230a 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2340,6 +2340,8 @@ int uart_register_driver(struct uart_driver *drv)
 	if (retval >= 0)
 		return retval;
 
+	for (i = 0; i < drv->nr; i++)
+		tty_port_destroy(&drv->state[i].port);
 	put_tty_driver(normal);
 out_kfree:
 	kfree(drv->state);
@@ -2359,8 +2361,12 @@ out:
 void uart_unregister_driver(struct uart_driver *drv)
 {
 	struct tty_driver *p = drv->tty_driver;
+	unsigned int i;
+
 	tty_unregister_driver(p);
 	put_tty_driver(p);
+	for (i = 0; i < drv->nr; i++)
+		tty_port_destroy(&drv->state[i].port);
 	kfree(drv->state);
 	drv->state = NULL;
 	drv->tty_driver = NULL;
diff --git a/drivers/tty/synclink.c b/drivers/tty/synclink.c
index 70e3a52..e4b5c39 100644
--- a/drivers/tty/synclink.c
+++ b/drivers/tty/synclink.c
@@ -4425,6 +4425,7 @@ static void synclink_cleanup(void)
 		mgsl_release_resources(info);
 		tmp = info;
 		info = info->next_device;
+		tty_port_destroy(&tmp->port);
 		kfree(tmp);
 	}
 	
diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index b38e954..6e4c340 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -3645,8 +3645,10 @@ static void device_init(int adapter_num, struct pci_dev *pdev)
 	for (i=0; i < port_count; ++i) {
 		port_array[i] = alloc_dev(adapter_num, i, pdev);
 		if (port_array[i] == NULL) {
-			for (--i; i >= 0; --i)
+			for (--i; i >= 0; --i) {
+				tty_port_destroy(&port_array[i]->port);
 				kfree(port_array[i]);
+			}
 			return;
 		}
 	}
@@ -3773,6 +3775,7 @@ static void slgt_cleanup(void)
 			release_resources(info);
 		tmp = info;
 		info = info->next_device;
+		tty_port_destroy(&tmp->port);
 		kfree(tmp);
 	}
 
diff --git a/drivers/tty/synclinkmp.c b/drivers/tty/synclinkmp.c
index f17d9f3..40745be 100644
--- a/drivers/tty/synclinkmp.c
+++ b/drivers/tty/synclinkmp.c
@@ -3843,8 +3843,10 @@ static void device_init(int adapter_num, struct pci_dev *pdev)
 	for ( port = 0; port < SCA_MAX_PORTS; ++port ) {
 		port_array[port] = alloc_dev(adapter_num,port,pdev);
 		if( port_array[port] == NULL ) {
-			for ( --port; port >= 0; --port )
+			for (--port; port >= 0; --port) {
+				tty_port_destroy(&port_array[port]->port);
 				kfree(port_array[port]);
+			}
 			return;
 		}
 	}
@@ -3953,6 +3955,7 @@ static void synclinkmp_cleanup(void)
 		}
 		tmp = info;
 		info = info->next_device;
+		tty_port_destroy(&tmp->port);
 		kfree(tmp);
 	}
 
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index f87d7e8..607636b 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -779,6 +779,7 @@ int vc_allocate(unsigned int currcons)	/* return 0 on success */
 		con_set_default_unimap(vc);
 	    vc->vc_screenbuf = kmalloc(vc->vc_screenbuf_size, GFP_KERNEL);
 	    if (!vc->vc_screenbuf) {
+		tty_port_destroy(&vc->port);
 		kfree(vc);
 		vc_cons[currcons].d = NULL;
 		return -ENOMEM;
@@ -999,8 +1000,10 @@ void vc_deallocate(unsigned int currcons)
 		put_pid(vc->vt_pid);
 		module_put(vc->vc_sw->owner);
 		kfree(vc->vc_screenbuf);
-		if (currcons >= MIN_NR_CONSOLES)
+		if (currcons >= MIN_NR_CONSOLES) {
+			tty_port_destroy(&vc->port);
 			kfree(vc);
+		}
 		vc_cons[currcons].d = NULL;
 	}
 }
diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
index f173952..d0f9548 100644
--- a/drivers/usb/gadget/u_serial.c
+++ b/drivers/usb/gadget/u_serial.c
@@ -1145,8 +1145,10 @@ int gserial_setup(struct usb_gadget *g, unsigned count)
 
 	return status;
 fail:
-	while (count--)
+	while (count--) {
+		tty_port_destroy(&ports[count].port->port);
 		kfree(ports[count].port);
+	}
 	put_tty_driver(gs_tty_driver);
 	gs_tty_driver = NULL;
 	return status;
@@ -1200,6 +1202,7 @@ void gserial_cleanup(void)
 
 		WARN_ON(port->port_usb != NULL);
 
+		tty_port_destroy(&port->port);
 		kfree(port);
 	}
 	n_ports = 0;
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 73b8e05..64bda13 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -597,6 +597,7 @@ static void port_release(struct device *dev)
 	kfifo_free(&port->write_fifo);
 	kfree(port->interrupt_in_buffer);
 	kfree(port->interrupt_out_buffer);
+	tty_port_destroy(&port->port);
 	kfree(port);
 }
 
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 496ce2c..a68c88c 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -183,6 +183,7 @@ static void __exit __ircomm_tty_cleanup(struct ircomm_tty_cb *self)
 	ircomm_tty_shutdown(self);
 
 	self->magic = 0;
+	tty_port_destroy(&self->port);
 	kfree(self);
 }
 
-- 
1.8.0



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/9] TTY: memory leaks patchset
  2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
                   ` (8 preceding siblings ...)
  2012-11-15  8:49 ` [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers Jiri Slaby
@ 2012-11-15 10:42 ` Alan Cox
  2012-11-16  1:21   ` Greg KH
  9 siblings, 1 reply; 20+ messages in thread
From: Alan Cox @ 2012-11-15 10:42 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: gregkh, alan, linux-kernel

> This series should go upstream regardless "TTY: move tty buffers to
> tty_port" if we decide to revert that commit due to the reported
> warnings I cannot reproduce and neither catch the root cause.

They seem to be warnings only and very obscure cases. I'd favour that
patch not being reverted simply because the only way we'll find a pattern
is by a lot more reports - and it doesn't seem to be anything but an
annoying log splat.

Alan

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/9] TTY: memory leaks patchset
  2012-11-15 10:42 ` [PATCH 0/9] TTY: memory leaks patchset Alan Cox
@ 2012-11-16  1:21   ` Greg KH
  0 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2012-11-16  1:21 UTC (permalink / raw)
  To: Alan Cox; +Cc: Jiri Slaby, alan, linux-kernel

On Thu, Nov 15, 2012 at 10:42:54AM +0000, Alan Cox wrote:
> > This series should go upstream regardless "TTY: move tty buffers to
> > tty_port" if we decide to revert that commit due to the reported
> > warnings I cannot reproduce and neither catch the root cause.
> 
> They seem to be warnings only and very obscure cases. I'd favour that
> patch not being reverted simply because the only way we'll find a pattern
> is by a lot more reports - and it doesn't seem to be anything but an
> annoying log splat.

I agree, it should stay to help catch any further problems that haven't
been fixed up yet.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers
  2012-11-15  8:49 ` [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers Jiri Slaby
@ 2012-11-18 13:39   ` Tilman Schmidt
  2012-11-27 16:52   ` Peter Hurley
  1 sibling, 0 replies; 20+ messages in thread
From: Tilman Schmidt @ 2012-11-18 13:39 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: gregkh, alan, linux-kernel, jirislaby

[-- Attachment #1: Type: text/plain, Size: 2433 bytes --]

Hi Jiri,

Two remarks wrt the Gigaset driver.

Am 15.11.2012 09:49, schrieb Jiri Slaby:
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index 30a6b17..bc9d89a 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -518,6 +518,7 @@ f_bcs:	gig_dbg(DEBUG_INIT, "freeing bcs[]");
>  	kfree(cs->bcs);
>  f_cs:	gig_dbg(DEBUG_INIT, "freeing cs");
>  	mutex_unlock(&cs->mutex);
> +	tty_port_destroy(&cs->port);
>  	free_cs(cs);
>  }
>  EXPORT_SYMBOL_GPL(gigaset_freecs);

It is not ok to call tty_port_destroy() unconditionally here.
gigaset_freecs() may be called from gigaset_initcs() before
the tty_port_init(&cs->port) call if a memory allocation fails.
This is best fixed by moving that call to case 1 of the preceding
switch statement because cs_init >= 1 covers exactly the cases
where the tty_port_init(&cs->port) call has already been passed.

> @@ -751,14 +752,14 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  	gig_dbg(DEBUG_INIT, "setting up iif");
>  	if (gigaset_isdn_regdev(cs, modulename) < 0) {
>  		pr_err("error registering ISDN device\n");
> -		goto error;
> +		goto error_port;
>  	}
>  
>  	make_valid(cs, VALID_ID);
>  	++cs->cs_init;
>  	gig_dbg(DEBUG_INIT, "setting up hw");
>  	if (cs->ops->initcshw(cs) < 0)
> -		goto error;
> +		goto error_port;
>  
>  	++cs->cs_init;
>  
> @@ -773,7 +774,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  		gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
>  		if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
>  			pr_err("could not allocate channel %d data\n", i);
> -			goto error;
> +			goto error_port;
>  		}
>  	}
>  
> @@ -786,7 +787,8 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  
>  	gig_dbg(DEBUG_INIT, "cs initialized");
>  	return cs;
> -
> +error_port:
> +	tty_port_destroy(&cs->port);
>  error:
>  	gig_dbg(DEBUG_INIT, "failed");
>  	gigaset_freecs(cs);

You have already added a tty_port_destroy() call to gigaset_freecs(cs)
above. Adding another one here will lead to the port being destroyed
twice in this code path.

Thanks,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers
  2012-11-15  8:49 ` [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers Jiri Slaby
  2012-11-18 13:39   ` Tilman Schmidt
@ 2012-11-27 16:52   ` Peter Hurley
  2012-11-27 17:04     ` Greg Kroah-Hartman
  1 sibling, 1 reply; 20+ messages in thread
From: Peter Hurley @ 2012-11-27 16:52 UTC (permalink / raw)
  To: Jiri Slaby, Greg Kroah-Hartman, Alan Cox; +Cc: linux-kernel, jirislaby

On Thu, 2012-11-15 at 09:49 +0100, Jiri Slaby wrote:
> After commit "TTY: move tty buffers to tty_port", the tty buffers are
> not freed in some drivers. This is because tty_port_destructor is not
> called whenever a tty_port is freed. This was an assumption I counted
> with but was unfortunately untrue. So fix the drivers to fulfil this
> assumption.
> 
> To be sure, the TTY buffers (and later some stuff) are gone along with
> the tty_port, we have to call tty_port_destroy at tear-down places.
> This is mostly where the structure containing a tty_port is freed.
> This patch does exactly that -- put tty_port_destroy at those places.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>

Hi Jiri & Greg & Alan,

I'd be more than willing to fixup staging/fwserial against this series
but I'm unclear on the mechanics since this series isn't in staging-next
and staging/fwserial isn't in tty-next.

Regards,
Peter Hurley




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers
  2012-11-27 16:52   ` Peter Hurley
@ 2012-11-27 17:04     ` Greg Kroah-Hartman
  2012-11-27 17:23       ` Peter Hurley
  2012-11-28  2:37       ` [PATCH -next 0/3] staging/fwserial: teardown cleanup Peter Hurley
  0 siblings, 2 replies; 20+ messages in thread
From: Greg Kroah-Hartman @ 2012-11-27 17:04 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Jiri Slaby, Alan Cox, linux-kernel, jirislaby

On Tue, Nov 27, 2012 at 11:52:09AM -0500, Peter Hurley wrote:
> On Thu, 2012-11-15 at 09:49 +0100, Jiri Slaby wrote:
> > After commit "TTY: move tty buffers to tty_port", the tty buffers are
> > not freed in some drivers. This is because tty_port_destructor is not
> > called whenever a tty_port is freed. This was an assumption I counted
> > with but was unfortunately untrue. So fix the drivers to fulfil this
> > assumption.
> > 
> > To be sure, the TTY buffers (and later some stuff) are gone along with
> > the tty_port, we have to call tty_port_destroy at tear-down places.
> > This is mostly where the structure containing a tty_port is freed.
> > This patch does exactly that -- put tty_port_destroy at those places.
> > 
> > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> 
> Hi Jiri & Greg & Alan,
> 
> I'd be more than willing to fixup staging/fwserial against this series
> but I'm unclear on the mechanics since this series isn't in staging-next
> and staging/fwserial isn't in tty-next.

The series is in tty-next, right?  Make it against linux-next, which has
the trees combined, and I'll work to figure it out when/where to apply
it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers
  2012-11-27 17:04     ` Greg Kroah-Hartman
@ 2012-11-27 17:23       ` Peter Hurley
  2012-11-28  2:37       ` [PATCH -next 0/3] staging/fwserial: teardown cleanup Peter Hurley
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Hurley @ 2012-11-27 17:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, Alan Cox, linux-kernel, jirislaby

On Tue, 2012-11-27 at 09:04 -0800, Greg Kroah-Hartman wrote:
> On Tue, Nov 27, 2012 at 11:52:09AM -0500, Peter Hurley wrote:
> > On Thu, 2012-11-15 at 09:49 +0100, Jiri Slaby wrote:
> > > After commit "TTY: move tty buffers to tty_port", the tty buffers are
> > > not freed in some drivers. This is because tty_port_destructor is not
> > > called whenever a tty_port is freed. This was an assumption I counted
> > > with but was unfortunately untrue. So fix the drivers to fulfil this
> > > assumption.
> > > 
> > > To be sure, the TTY buffers (and later some stuff) are gone along with
> > > the tty_port, we have to call tty_port_destroy at tear-down places.
> > > This is mostly where the structure containing a tty_port is freed.
> > > This patch does exactly that -- put tty_port_destroy at those places.
> > > 
> > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > 
> > Hi Jiri & Greg & Alan,
> > 
> > I'd be more than willing to fixup staging/fwserial against this series
> > but I'm unclear on the mechanics since this series isn't in staging-next
> > and staging/fwserial isn't in tty-next.
> 
> The series is in tty-next, right?

Yes.

> Make it against linux-next, which has
> the trees combined, and I'll work to figure it out when/where to apply
> it.

Ok, will do.

Thanks,
Peter



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH -next 0/3] staging/fwserial: teardown cleanup
  2012-11-27 17:04     ` Greg Kroah-Hartman
  2012-11-27 17:23       ` Peter Hurley
@ 2012-11-28  2:37       ` Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 1/3] staging/fwserial: Destruct embedded tty_port on teardown Peter Hurley
                           ` (2 more replies)
  1 sibling, 3 replies; 20+ messages in thread
From: Peter Hurley @ 2012-11-28  2:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Peter Hurley

Hi Greg,

This patch series fixes up staging/fwserial to meet the new requirements
in Jiri Slaby's series "TTY: memory leaks patchset" (in tty-next).

Strictly speaking, only PATCH 1/3 implements the necessary cleanup.
PATCHES 2/3 & 3/3 are additional cleanups in the same area.

Peter Hurley (3):
  staging/fwserial: Destruct embedded tty_port on teardown
  staging/fwserial: Use WARN_ONCE when port table is corrupted
  staging/fwserial: Remove superfluous free

 drivers/staging/fwserial/fwserial.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

-- 
1.8.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH -next 1/3] staging/fwserial: Destruct embedded tty_port on teardown
  2012-11-28  2:37       ` [PATCH -next 0/3] staging/fwserial: teardown cleanup Peter Hurley
@ 2012-11-28  2:37         ` Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 2/3] staging/fwserial: Use WARN_ONCE when port table is corrupted Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 3/3] staging/fwserial: Remove superfluous free Peter Hurley
  2 siblings, 0 replies; 20+ messages in thread
From: Peter Hurley @ 2012-11-28  2:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Peter Hurley, Jiri Slaby, Alan Cox

For TTY drivers that manage the port lifetime, the tty_port should
to be specifically destructed when the port lifetime ends. Now that
a method has been added to do this, use it.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@linux.intel.com>
---
 drivers/staging/fwserial/fwserial.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 5d4d64a..99a2d2d 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -955,6 +955,7 @@ static void fwserial_destroy(struct kref *kref)
 	for (j = 0; j < num_ports; ++j) {
 		fw_core_remove_address_handler(&ports[j]->rx_handler);
 		dma_fifo_free(&ports[j]->tx_fifo);
+		tty_port_destroy(&ports[j]->port);
 		kfree(ports[j]);
 	}
 	kfree(serial);
@@ -2369,8 +2370,10 @@ unregister_ttys:
 	return err;
 
 free_ports:
-	for (--i; i >= 0; --i)
+	for (--i; i >= 0; --i) {
+		tty_port_destroy(&serial->ports[i]->port);
 		kfree(serial->ports[i]);
+	}
 	kfree(serial);
 	return err;
 }
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH -next 2/3] staging/fwserial: Use WARN_ONCE when port table is corrupted
  2012-11-28  2:37       ` [PATCH -next 0/3] staging/fwserial: teardown cleanup Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 1/3] staging/fwserial: Destruct embedded tty_port on teardown Peter Hurley
@ 2012-11-28  2:37         ` Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 3/3] staging/fwserial: Remove superfluous free Peter Hurley
  2 siblings, 0 replies; 20+ messages in thread
From: Peter Hurley @ 2012-11-28  2:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Peter Hurley


Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 99a2d2d..0681967 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -939,14 +939,9 @@ static void fwserial_destroy(struct kref *kref)
 
 	mutex_lock(&port_table_lock);
 	for (j = 0; j < num_ports; ++i, ++j) {
-		static bool once;
-		int corrupt = port_table[i] != ports[j];
-		if (corrupt && !once) {
-			WARN(corrupt, "port_table[%d]: %p != ports[%d]: %p",
-			     i, port_table[i], j, ports[j]);
-			once = true;
-			port_table_corrupt = true;
-		}
+		port_table_corrupt |= port_table[i] != ports[j];
+		WARN_ONCE(port_table_corrupt, "port_table[%d]: %p != ports[%d]: %p",
+		     i, port_table[i], j, ports[j]);
 
 		port_table[i] = NULL;
 	}
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH -next 3/3] staging/fwserial: Remove superfluous free
  2012-11-28  2:37       ` [PATCH -next 0/3] staging/fwserial: teardown cleanup Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 1/3] staging/fwserial: Destruct embedded tty_port on teardown Peter Hurley
  2012-11-28  2:37         ` [PATCH -next 2/3] staging/fwserial: Use WARN_ONCE when port table is corrupted Peter Hurley
@ 2012-11-28  2:37         ` Peter Hurley
  2 siblings, 0 replies; 20+ messages in thread
From: Peter Hurley @ 2012-11-28  2:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-kernel, Peter Hurley

Now that the dma fifo is allocated on activate and freed on
shutdown, this extra free is harmless but unnecessary.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 0681967..61ee290 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -949,7 +949,6 @@ static void fwserial_destroy(struct kref *kref)
 
 	for (j = 0; j < num_ports; ++j) {
 		fw_core_remove_address_handler(&ports[j]->rx_handler);
-		dma_fifo_free(&ports[j]->tx_fifo);
 		tty_port_destroy(&ports[j]->port);
 		kfree(ports[j]);
 	}
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-11-28  2:37 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-15  8:49 [PATCH 0/9] TTY: memory leaks patchset Jiri Slaby
2012-11-15  8:49 ` [PATCH 1/9] TTY: isicom, stop using port->tty Jiri Slaby
2012-11-15  8:49 ` [PATCH 2/9] TTY: pty, fix tty buffers leak Jiri Slaby
2012-11-15  8:49 ` [PATCH 3/9] ISDN: capi, use kref from tty_port Jiri Slaby
2012-11-15  8:49 ` [PATCH 4/9] MMC: sdio_uart, remove unused member from sdio_uart_port Jiri Slaby
2012-11-15  8:49 ` [PATCH 5/9] MMC: sdio, use kref from tty_port Jiri Slaby
2012-11-15  8:49 ` [PATCH 6/9] TTY: n_gsm, " Jiri Slaby
2012-11-15  8:49 ` [PATCH 7/9] TTY: introduce tty_port_destroy Jiri Slaby
2012-11-15  8:49 ` [PATCH 8/9] TTY: isicom, fix tty buffers memory leak Jiri Slaby
2012-11-15  8:49 ` [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers Jiri Slaby
2012-11-18 13:39   ` Tilman Schmidt
2012-11-27 16:52   ` Peter Hurley
2012-11-27 17:04     ` Greg Kroah-Hartman
2012-11-27 17:23       ` Peter Hurley
2012-11-28  2:37       ` [PATCH -next 0/3] staging/fwserial: teardown cleanup Peter Hurley
2012-11-28  2:37         ` [PATCH -next 1/3] staging/fwserial: Destruct embedded tty_port on teardown Peter Hurley
2012-11-28  2:37         ` [PATCH -next 2/3] staging/fwserial: Use WARN_ONCE when port table is corrupted Peter Hurley
2012-11-28  2:37         ` [PATCH -next 3/3] staging/fwserial: Remove superfluous free Peter Hurley
2012-11-15 10:42 ` [PATCH 0/9] TTY: memory leaks patchset Alan Cox
2012-11-16  1:21   ` Greg KH

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.