From: <gregkh@linuxfoundation.org>
To: alex.popov@linux.com, gregkh@linuxfoundation.org
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "tty: n_hdlc: get rid of racy n_hdlc.tbuf" has been added to the 4.4-stable tree
Date: Sun, 12 Mar 2017 15:44:16 +0100 [thread overview]
Message-ID: <14893298562230@kroah.com> (raw)
This is a note to let you know that I've just added the patch titled
tty: n_hdlc: get rid of racy n_hdlc.tbuf
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
tty-n_hdlc-get-rid-of-racy-n_hdlc.tbuf.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From 82f2341c94d270421f383641b7cd670e474db56b Mon Sep 17 00:00:00 2001
From: Alexander Popov <alex.popov@linux.com>
Date: Tue, 28 Feb 2017 19:54:40 +0300
Subject: tty: n_hdlc: get rid of racy n_hdlc.tbuf
From: Alexander Popov <alex.popov@linux.com>
commit 82f2341c94d270421f383641b7cd670e474db56b upstream.
Currently N_HDLC line discipline uses a self-made singly linked list for
data buffers and has n_hdlc.tbuf pointer for buffer retransmitting after
an error.
The commit be10eb7589337e5defbe214dae038a53dd21add8
("tty: n_hdlc add buffer flushing") introduced racy access to n_hdlc.tbuf.
After tx error concurrent flush_tx_queue() and n_hdlc_send_frames() can put
one data buffer to tx_free_buf_list twice. That causes double free in
n_hdlc_release().
Let's use standard kernel linked list and get rid of n_hdlc.tbuf:
in case of tx error put current data buffer after the head of tx_buf_list.
Signed-off-by: Alexander Popov <alex.popov@linux.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/tty/n_hdlc.c | 132 ++++++++++++++++++++++++++-------------------------
1 file changed, 69 insertions(+), 63 deletions(-)
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -114,7 +114,7 @@
#define DEFAULT_TX_BUF_COUNT 3
struct n_hdlc_buf {
- struct n_hdlc_buf *link;
+ struct list_head list_item;
int count;
char buf[1];
};
@@ -122,8 +122,7 @@ struct n_hdlc_buf {
#define N_HDLC_BUF_SIZE (sizeof(struct n_hdlc_buf) + maxframe)
struct n_hdlc_buf_list {
- struct n_hdlc_buf *head;
- struct n_hdlc_buf *tail;
+ struct list_head list;
int count;
spinlock_t spinlock;
};
@@ -136,7 +135,6 @@ struct n_hdlc_buf_list {
* @backup_tty - TTY to use if tty gets closed
* @tbusy - reentrancy flag for tx wakeup code
* @woke_up - FIXME: describe this field
- * @tbuf - currently transmitting tx buffer
* @tx_buf_list - list of pending transmit frame buffers
* @rx_buf_list - list of received frame buffers
* @tx_free_buf_list - list unused transmit frame buffers
@@ -149,7 +147,6 @@ struct n_hdlc {
struct tty_struct *backup_tty;
int tbusy;
int woke_up;
- struct n_hdlc_buf *tbuf;
struct n_hdlc_buf_list tx_buf_list;
struct n_hdlc_buf_list rx_buf_list;
struct n_hdlc_buf_list tx_free_buf_list;
@@ -159,6 +156,8 @@ struct n_hdlc {
/*
* HDLC buffer list manipulation functions
*/
+static void n_hdlc_buf_return(struct n_hdlc_buf_list *buf_list,
+ struct n_hdlc_buf *buf);
static void n_hdlc_buf_put(struct n_hdlc_buf_list *list,
struct n_hdlc_buf *buf);
static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *list);
@@ -208,16 +207,9 @@ static void flush_tx_queue(struct tty_st
{
struct n_hdlc *n_hdlc = tty2n_hdlc(tty);
struct n_hdlc_buf *buf;
- unsigned long flags;
while ((buf = n_hdlc_buf_get(&n_hdlc->tx_buf_list)))
n_hdlc_buf_put(&n_hdlc->tx_free_buf_list, buf);
- spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock, flags);
- if (n_hdlc->tbuf) {
- n_hdlc_buf_put(&n_hdlc->tx_free_buf_list, n_hdlc->tbuf);
- n_hdlc->tbuf = NULL;
- }
- spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
}
static struct tty_ldisc_ops n_hdlc_ldisc = {
@@ -283,7 +275,6 @@ static void n_hdlc_release(struct n_hdlc
} else
break;
}
- kfree(n_hdlc->tbuf);
kfree(n_hdlc);
} /* end of n_hdlc_release() */
@@ -402,13 +393,7 @@ static void n_hdlc_send_frames(struct n_
n_hdlc->woke_up = 0;
spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock, flags);
- /* get current transmit buffer or get new transmit */
- /* buffer from list of pending transmit buffers */
-
- tbuf = n_hdlc->tbuf;
- if (!tbuf)
- tbuf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
-
+ tbuf = n_hdlc_buf_get(&n_hdlc->tx_buf_list);
while (tbuf) {
if (debuglevel >= DEBUG_LEVEL_INFO)
printk("%s(%d)sending frame %p, count=%d\n",
@@ -420,7 +405,7 @@ static void n_hdlc_send_frames(struct n_
/* rollback was possible and has been done */
if (actual == -ERESTARTSYS) {
- n_hdlc->tbuf = tbuf;
+ n_hdlc_buf_return(&n_hdlc->tx_buf_list, tbuf);
break;
}
/* if transmit error, throw frame away by */
@@ -435,10 +420,7 @@ static void n_hdlc_send_frames(struct n_
/* free current transmit buffer */
n_hdlc_buf_put(&n_hdlc->tx_free_buf_list, tbuf);
-
- /* this tx buffer is done */
- n_hdlc->tbuf = NULL;
-
+
/* wait up sleeping writers */
wake_up_interruptible(&tty->write_wait);
@@ -448,10 +430,12 @@ static void n_hdlc_send_frames(struct n_
if (debuglevel >= DEBUG_LEVEL_INFO)
printk("%s(%d)frame %p pending\n",
__FILE__,__LINE__,tbuf);
-
- /* buffer not accepted by driver */
- /* set this buffer as pending buffer */
- n_hdlc->tbuf = tbuf;
+
+ /*
+ * the buffer was not accepted by driver,
+ * return it back into tx queue
+ */
+ n_hdlc_buf_return(&n_hdlc->tx_buf_list, tbuf);
break;
}
}
@@ -749,7 +733,8 @@ static int n_hdlc_tty_ioctl(struct tty_s
int error = 0;
int count;
unsigned long flags;
-
+ struct n_hdlc_buf *buf = NULL;
+
if (debuglevel >= DEBUG_LEVEL_INFO)
printk("%s(%d)n_hdlc_tty_ioctl() called %d\n",
__FILE__,__LINE__,cmd);
@@ -763,8 +748,10 @@ static int n_hdlc_tty_ioctl(struct tty_s
/* report count of read data available */
/* in next available frame (if any) */
spin_lock_irqsave(&n_hdlc->rx_buf_list.spinlock,flags);
- if (n_hdlc->rx_buf_list.head)
- count = n_hdlc->rx_buf_list.head->count;
+ buf = list_first_entry_or_null(&n_hdlc->rx_buf_list.list,
+ struct n_hdlc_buf, list_item);
+ if (buf)
+ count = buf->count;
else
count = 0;
spin_unlock_irqrestore(&n_hdlc->rx_buf_list.spinlock,flags);
@@ -776,8 +763,10 @@ static int n_hdlc_tty_ioctl(struct tty_s
count = tty_chars_in_buffer(tty);
/* add size of next output frame in queue */
spin_lock_irqsave(&n_hdlc->tx_buf_list.spinlock,flags);
- if (n_hdlc->tx_buf_list.head)
- count += n_hdlc->tx_buf_list.head->count;
+ buf = list_first_entry_or_null(&n_hdlc->tx_buf_list.list,
+ struct n_hdlc_buf, list_item);
+ if (buf)
+ count += buf->count;
spin_unlock_irqrestore(&n_hdlc->tx_buf_list.spinlock,flags);
error = put_user(count, (int __user *)arg);
break;
@@ -825,14 +814,14 @@ static unsigned int n_hdlc_tty_poll(stru
poll_wait(filp, &tty->write_wait, wait);
/* set bits for operations that won't block */
- if (n_hdlc->rx_buf_list.head)
+ if (!list_empty(&n_hdlc->rx_buf_list.list))
mask |= POLLIN | POLLRDNORM; /* readable */
if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
mask |= POLLHUP;
if (tty_hung_up_p(filp))
mask |= POLLHUP;
if (!tty_is_writelocked(tty) &&
- n_hdlc->tx_free_buf_list.head)
+ !list_empty(&n_hdlc->tx_free_buf_list.list))
mask |= POLLOUT | POLLWRNORM; /* writable */
}
return mask;
@@ -856,7 +845,12 @@ static struct n_hdlc *n_hdlc_alloc(void)
spin_lock_init(&n_hdlc->tx_free_buf_list.spinlock);
spin_lock_init(&n_hdlc->rx_buf_list.spinlock);
spin_lock_init(&n_hdlc->tx_buf_list.spinlock);
-
+
+ INIT_LIST_HEAD(&n_hdlc->rx_free_buf_list.list);
+ INIT_LIST_HEAD(&n_hdlc->tx_free_buf_list.list);
+ INIT_LIST_HEAD(&n_hdlc->rx_buf_list.list);
+ INIT_LIST_HEAD(&n_hdlc->tx_buf_list.list);
+
/* allocate free rx buffer list */
for(i=0;i<DEFAULT_RX_BUF_COUNT;i++) {
buf = kmalloc(N_HDLC_BUF_SIZE, GFP_KERNEL);
@@ -884,53 +878,65 @@ static struct n_hdlc *n_hdlc_alloc(void)
} /* end of n_hdlc_alloc() */
/**
+ * n_hdlc_buf_return - put the HDLC buffer after the head of the specified list
+ * @buf_list - pointer to the buffer list
+ * @buf - pointer to the buffer
+ */
+static void n_hdlc_buf_return(struct n_hdlc_buf_list *buf_list,
+ struct n_hdlc_buf *buf)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&buf_list->spinlock, flags);
+
+ list_add(&buf->list_item, &buf_list->list);
+ buf_list->count++;
+
+ spin_unlock_irqrestore(&buf_list->spinlock, flags);
+}
+
+/**
* n_hdlc_buf_put - add specified HDLC buffer to tail of specified list
- * @list - pointer to buffer list
+ * @buf_list - pointer to buffer list
* @buf - pointer to buffer
*/
-static void n_hdlc_buf_put(struct n_hdlc_buf_list *list,
+static void n_hdlc_buf_put(struct n_hdlc_buf_list *buf_list,
struct n_hdlc_buf *buf)
{
unsigned long flags;
- spin_lock_irqsave(&list->spinlock,flags);
-
- buf->link=NULL;
- if (list->tail)
- list->tail->link = buf;
- else
- list->head = buf;
- list->tail = buf;
- (list->count)++;
-
- spin_unlock_irqrestore(&list->spinlock,flags);
-
+
+ spin_lock_irqsave(&buf_list->spinlock, flags);
+
+ list_add_tail(&buf->list_item, &buf_list->list);
+ buf_list->count++;
+
+ spin_unlock_irqrestore(&buf_list->spinlock, flags);
} /* end of n_hdlc_buf_put() */
/**
* n_hdlc_buf_get - remove and return an HDLC buffer from list
- * @list - pointer to HDLC buffer list
+ * @buf_list - pointer to HDLC buffer list
*
* Remove and return an HDLC buffer from the head of the specified HDLC buffer
* list.
* Returns a pointer to HDLC buffer if available, otherwise %NULL.
*/
-static struct n_hdlc_buf* n_hdlc_buf_get(struct n_hdlc_buf_list *list)
+static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)
{
unsigned long flags;
struct n_hdlc_buf *buf;
- spin_lock_irqsave(&list->spinlock,flags);
-
- buf = list->head;
+
+ spin_lock_irqsave(&buf_list->spinlock, flags);
+
+ buf = list_first_entry_or_null(&buf_list->list,
+ struct n_hdlc_buf, list_item);
if (buf) {
- list->head = buf->link;
- (list->count)--;
+ list_del(&buf->list_item);
+ buf_list->count--;
}
- if (!list->head)
- list->tail = NULL;
-
- spin_unlock_irqrestore(&list->spinlock,flags);
+
+ spin_unlock_irqrestore(&buf_list->spinlock, flags);
return buf;
-
} /* end of n_hdlc_buf_get() */
static char hdlc_banner[] __initdata =
Patches currently in stable-queue which might be from alex.popov@linux.com are
queue-4.4/tty-n_hdlc-get-rid-of-racy-n_hdlc.tbuf.patch
reply other threads:[~2017-03-12 14:44 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=14893298562230@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=alex.popov@linux.com \
--cc=stable-commits@vger.kernel.org \
--cc=stable@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 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.