linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pcmcia: synclink_cs: fix potential tty NULL dereference
@ 2012-09-13 21:30 Alexey Khoroshilov
  2012-09-14 10:49 ` Alan Cox
  0 siblings, 1 reply; 3+ messages in thread
From: Alexey Khoroshilov @ 2012-09-13 21:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Alexey Khoroshilov, linux-kernel, ldv-project

tty_port_tty_get() can return NULL after port hangup that may happen anytime.
The patch adds checks that tty_port_tty_get() returns nonNULL around places
where tty is actually used.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/char/pcmcia/synclink_cs.c |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 0a484b4..13c9747 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -891,6 +891,9 @@ static void rx_ready_async(MGSLPC_INFO *info, int tcd, struct tty_struct *tty)
 	int work = 0;
  	struct mgsl_icount *icount = &info->icount;
 
+	if (!tty)
+		return;
+
 	if (tcd) {
 		/* early termination, get FIFO count from RBCL register */
 		fifo_count = (unsigned char)(read_reg(info, CHA+RBCL) & 0x1f);
@@ -980,7 +983,7 @@ static void tx_done(MGSLPC_INFO *info, struct tty_struct *tty)
 	else
 #endif
 	{
-		if (tty->stopped || tty->hw_stopped) {
+		if (tty && (tty->stopped || tty->hw_stopped)) {
 			tx_stop(info);
 			return;
 		}
@@ -1000,7 +1003,7 @@ static void tx_ready(MGSLPC_INFO *info, struct tty_struct *tty)
 		if (!info->tx_active)
 			return;
 	} else {
-		if (tty->stopped || tty->hw_stopped) {
+		if (tty && (tty->stopped || tty->hw_stopped)) {
 			tx_stop(info);
 			return;
 		}
@@ -1050,13 +1053,12 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
 	wake_up_interruptible(&info->status_event_wait_q);
 	wake_up_interruptible(&info->event_wait_q);
 
-	if (info->port.flags & ASYNC_CTS_FLOW) {
+	if (tty && (info->port.flags & ASYNC_CTS_FLOW)) {
 		if (tty->hw_stopped) {
 			if (info->serial_signals & SerialSignal_CTS) {
 				if (debug_level >= DEBUG_LEVEL_ISR)
 					printk("CTS tx start...");
-				if (tty)
-					tty->hw_stopped = 0;
+				tty->hw_stopped = 0;
 				tx_start(info, tty);
 				info->pending_bh |= BH_TRANSMIT;
 				return;
@@ -1065,8 +1067,7 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
 			if (!(info->serial_signals & SerialSignal_CTS)) {
 				if (debug_level >= DEBUG_LEVEL_ISR)
 					printk("CTS tx stop...");
-				if (tty)
-					tty->hw_stopped = 1;
+				tty->hw_stopped = 1;
 				tx_stop(info);
 			}
 		}
-- 
1.7.9.5


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

* Re: [PATCH] pcmcia: synclink_cs: fix potential tty NULL dereference
  2012-09-13 21:30 [PATCH] pcmcia: synclink_cs: fix potential tty NULL dereference Alexey Khoroshilov
@ 2012-09-14 10:49 ` Alan Cox
  2012-09-14 21:29   ` [PATCH v2] " Alexey Khoroshilov
  0 siblings, 1 reply; 3+ messages in thread
From: Alan Cox @ 2012-09-14 10:49 UTC (permalink / raw)
  To: Alexey Khoroshilov; +Cc: Greg Kroah-Hartman, linux-kernel, ldv-project

On Fri, 14 Sep 2012 01:30:06 +0400
Alexey Khoroshilov <khoroshilov@ispras.ru> wrote:

> tty_port_tty_get() can return NULL after port hangup that may happen anytime.
> The patch adds checks that tty_port_tty_get() returns nonNULL around places
> where tty is actually used.

I don't believe you can simply skip the processing in this case howevver
on the rx side. You are no longer reading the state, draining the FIFO
etc. Have you tested this on actual hardware and faked the tty = NULL
case ?

The tx one looks better.

Alan

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

* [PATCH v2] pcmcia: synclink_cs: fix potential tty NULL dereference
  2012-09-14 10:49 ` Alan Cox
@ 2012-09-14 21:29   ` Alexey Khoroshilov
  0 siblings, 0 replies; 3+ messages in thread
From: Alexey Khoroshilov @ 2012-09-14 21:29 UTC (permalink / raw)
  To: Alan Cox
  Cc: Alexey Khoroshilov, Greg Kroah-Hartman, linux-kernel, ldv-project

tty_port_tty_get() can return NULL after port hangup that may happen anytime.
The patch adds checks that tty_port_tty_get() returns nonNULL around places
where tty is actually used.

I have no actual hardware to test the patch, so I have updated rx side 
processing from common sense only.

v2: rx handling updated according Alan Cox feedback.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/char/pcmcia/synclink_cs.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 0a484b4..a6b8dde 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -891,6 +891,14 @@ static void rx_ready_async(MGSLPC_INFO *info, int tcd, struct tty_struct *tty)
 	int work = 0;
  	struct mgsl_icount *icount = &info->icount;
 
+	if (!tty) {
+		/* tty is not available anymore */
+		issue_command(info, CHA, CMD_RXRESET);
+		if (debug_level >= DEBUG_LEVEL_ISR)
+			printk("%s(%d):rx_ready_async(tty=NULL)\n",__FILE__,__LINE__);
+		return;
+	}
+
 	if (tcd) {
 		/* early termination, get FIFO count from RBCL register */
 		fifo_count = (unsigned char)(read_reg(info, CHA+RBCL) & 0x1f);
@@ -980,7 +988,7 @@ static void tx_done(MGSLPC_INFO *info, struct tty_struct *tty)
 	else
 #endif
 	{
-		if (tty->stopped || tty->hw_stopped) {
+		if (tty && (tty->stopped || tty->hw_stopped)) {
 			tx_stop(info);
 			return;
 		}
@@ -1000,7 +1008,7 @@ static void tx_ready(MGSLPC_INFO *info, struct tty_struct *tty)
 		if (!info->tx_active)
 			return;
 	} else {
-		if (tty->stopped || tty->hw_stopped) {
+		if (tty && (tty->stopped || tty->hw_stopped)) {
 			tx_stop(info);
 			return;
 		}
@@ -1050,13 +1058,12 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
 	wake_up_interruptible(&info->status_event_wait_q);
 	wake_up_interruptible(&info->event_wait_q);
 
-	if (info->port.flags & ASYNC_CTS_FLOW) {
+	if (tty && (info->port.flags & ASYNC_CTS_FLOW)) {
 		if (tty->hw_stopped) {
 			if (info->serial_signals & SerialSignal_CTS) {
 				if (debug_level >= DEBUG_LEVEL_ISR)
 					printk("CTS tx start...");
-				if (tty)
-					tty->hw_stopped = 0;
+				tty->hw_stopped = 0;
 				tx_start(info, tty);
 				info->pending_bh |= BH_TRANSMIT;
 				return;
@@ -1065,8 +1072,7 @@ static void cts_change(MGSLPC_INFO *info, struct tty_struct *tty)
 			if (!(info->serial_signals & SerialSignal_CTS)) {
 				if (debug_level >= DEBUG_LEVEL_ISR)
 					printk("CTS tx stop...");
-				if (tty)
-					tty->hw_stopped = 1;
+				tty->hw_stopped = 1;
 				tx_stop(info);
 			}
 		}
-- 
1.7.9.5


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

end of thread, other threads:[~2012-09-14 21:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-13 21:30 [PATCH] pcmcia: synclink_cs: fix potential tty NULL dereference Alexey Khoroshilov
2012-09-14 10:49 ` Alan Cox
2012-09-14 21:29   ` [PATCH v2] " Alexey Khoroshilov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).