linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: greg@kroah.com, linux-serial@vger.kernel.org
Subject: [PATCH 6/8] n_gsm: avoid accessing freed memory during CMD_FCOFF condition
Date: Mon, 13 Aug 2012 13:44:59 +0100	[thread overview]
Message-ID: <20120813124451.6125.35751.stgit@localhost.localdomain> (raw)
In-Reply-To: <20120813124254.6125.70371.stgit@localhost.localdomain>

From: Russ Gorby <russ.gorby@intel.com>

gsm_data_kick was recently modified to allow messages on the
tx queue bound for DLCI0 to flow even during FCOFF conditions.
Unfortunately we introduced a bug discovered by code inspection
where subsequent list traversers can access freed memory if
the DLCI0 messages were not all at the head of the list.

Replaced singly linked tx list w/ a list_head and used
provided interfaces for traversing and deleting members.

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
Tested-by: Yin, Fengwei <fengwei.yin@intel.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Cc: Riding School <stable@kernel.org>
---

 drivers/tty/n_gsm.c |   40 +++++++++++++---------------------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index 51ba2f2..e2bdb8b 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -108,7 +108,7 @@ struct gsm_mux_net {
  */
 
 struct gsm_msg {
-	struct gsm_msg *next;
+	struct list_head list;
 	u8 addr;		/* DLCI address + flags */
 	u8 ctrl;		/* Control byte + flags */
 	unsigned int len;	/* Length of data block (can be zero) */
@@ -245,8 +245,7 @@ struct gsm_mux {
 	unsigned int tx_bytes;		/* TX data outstanding */
 #define TX_THRESH_HI		8192
 #define TX_THRESH_LO		2048
-	struct gsm_msg *tx_head;	/* Pending data packets */
-	struct gsm_msg *tx_tail;
+	struct list_head tx_list;	/* Pending data packets */
 
 	/* Control messages */
 	struct timer_list t2_timer;	/* Retransmit timer for commands */
@@ -663,7 +662,7 @@ static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len,
 	m->len = len;
 	m->addr = addr;
 	m->ctrl = ctrl;
-	m->next = NULL;
+	INIT_LIST_HEAD(&m->list);
 	return m;
 }
 
@@ -681,16 +680,13 @@ static struct gsm_msg *gsm_data_alloc(struct gsm_mux *gsm, u8 addr, int len,
 
 static void gsm_data_kick(struct gsm_mux *gsm)
 {
-	struct gsm_msg *msg = gsm->tx_head;
-	struct gsm_msg *free_msg;
+	struct gsm_msg *msg, *nmsg;
 	int len;
 	int skip_sof = 0;
 
-	while (msg) {
-		if (gsm->constipated && msg->addr) {
-			msg = msg->next;
+	list_for_each_entry_safe(msg, nmsg, &gsm->tx_list, list) {
+		if (gsm->constipated && msg->addr)
 			continue;
-		}
 		if (gsm->encoding != 0) {
 			gsm->txframe[0] = GSM1_SOF;
 			len = gsm_stuff_frame(msg->data,
@@ -718,14 +714,9 @@ static void gsm_data_kick(struct gsm_mux *gsm)
 		   burst */
 		skip_sof = 1;
 
-		if (gsm->tx_head == msg)
-			gsm->tx_head = msg->next;
-		free_msg = msg;
-		msg = msg->next;
-		kfree(free_msg);
+		list_del(&msg->list);
+		kfree(msg);
 	}
-	if (!gsm->tx_head)
-		gsm->tx_tail = NULL;
 }
 
 /**
@@ -774,11 +765,7 @@ static void __gsm_data_queue(struct gsm_dlci *dlci, struct gsm_msg *msg)
 	msg->data = dp;
 
 	/* Add to the actual output queue */
-	if (gsm->tx_tail)
-		gsm->tx_tail->next = msg;
-	else
-		gsm->tx_head = msg;
-	gsm->tx_tail = msg;
+	list_add_tail(&msg->list, &gsm->tx_list);
 	gsm->tx_bytes += msg->len;
 	gsm_data_kick(gsm);
 }
@@ -2026,7 +2013,7 @@ void gsm_cleanup_mux(struct gsm_mux *gsm)
 {
 	int i;
 	struct gsm_dlci *dlci = gsm->dlci[0];
-	struct gsm_msg *txq;
+	struct gsm_msg *txq, *utxq;
 	struct gsm_control *gc;
 
 	gsm->dead = 1;
@@ -2061,11 +2048,9 @@ void gsm_cleanup_mux(struct gsm_mux *gsm)
 		if (gsm->dlci[i])
 			gsm_dlci_release(gsm->dlci[i]);
 	/* Now wipe the queues */
-	for (txq = gsm->tx_head; txq != NULL; txq = gsm->tx_head) {
-		gsm->tx_head = txq->next;
+	list_for_each_entry_safe(txq, ntxq, &gsm->tx_list, list)
 		kfree(txq);
-	}
-	gsm->tx_tail = NULL;
+	INIT_LIST_HEAD(&gsm->tx_list);
 }
 EXPORT_SYMBOL_GPL(gsm_cleanup_mux);
 
@@ -2176,6 +2161,7 @@ struct gsm_mux *gsm_alloc_mux(void)
 	}
 	spin_lock_init(&gsm->lock);
 	kref_init(&gsm->ref);
+	INIT_LIST_HEAD(&gsm->tx_list);
 
 	gsm->t1 = T1;
 	gsm->t2 = T2;


  parent reply	other threads:[~2012-08-13 12:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-13 12:43 [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX Alan Cox
2012-08-13 12:43 ` [PATCH 2/8] n_gsm: uplink SKBs accumulate on list Alan Cox
2012-08-13 12:43 ` [PATCH 3/8] n_gsm : Flow control handling in Mux driver Alan Cox
2012-08-13 12:44 ` [PATCH 4/8] char: n_gsm: remove message filtering for contipated DLCI Alan Cox
2012-08-13 12:44 ` [PATCH 5/8] n_gsm: added interlocking for gsm_data_lock for certain code paths Alan Cox
2012-08-13 12:44 ` Alan Cox [this message]
2012-08-13 12:45 ` [PATCH 7/8] n_gsm: replace kfree_skb w/ appropriate dev_* versions Alan Cox
2012-08-13 12:45 ` [PATCH 8/8] n_gsm: memory leak in uplink error path Alan Cox
2012-08-16 18:57 ` [PATCH 1/8] n_gsm.c: Implement 3GPP27.010 DLC start-up procedure in MUX Greg KH
2012-08-16 19:01 ` Greg KH
2012-08-16 19:12   ` Alan Cox
2012-08-16 19:17     ` Greg KH

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=20120813124451.6125.35751.stgit@localhost.localdomain \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=greg@kroah.com \
    --cc=linux-serial@vger.kernel.org \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).