From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Alexander Popov <alex.popov@linux.com>
Subject: [PATCH 4.4 02/36] tty: n_hdlc: get rid of racy n_hdlc.tbuf
Date: Mon, 13 Mar 2017 16:39:02 +0800 [thread overview]
Message-ID: <20170313083352.664793386@linuxfoundation.org> (raw)
In-Reply-To: <20170313083352.550085638@linuxfoundation.org>
4.4-stable review patch. If anyone has any objections, please let me know.
------------------
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 =
next prev parent reply other threads:[~2017-03-13 8:41 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-13 8:39 [PATCH 4.4 00/36] 4.4.54-stable review Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 01/36] TTY: n_hdlc, fix lockdep false positive Greg Kroah-Hartman
2017-03-13 8:39 ` Greg Kroah-Hartman [this message]
2017-03-13 8:39 ` [PATCH 4.4 03/36] serial: 8250_pci: Add MKS Tenta SCOM-0800 and SCOM-0801 cards Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 04/36] KVM: s390: Disable dirty log retrieval for UCONTROL guests Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 05/36] KVM: VMX: use correct vmcs_read/write for guest segment selector/base Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 06/36] Bluetooth: Add another AR3012 04ca:3018 device Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 07/36] s390/qdio: clear DSCI prior to scanning multiple input queues Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 08/36] s390/dcssblk: fix device size calculation in dcssblk_direct_access() Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 09/36] s390: TASK_SIZE for kernel threads Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 10/36] s390: make setup_randomness work Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 11/36] s390: use correct input data address for setup_randomness Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 12/36] net: mvpp2: fix DMA address calculation in mvpp2_txq_inc_put() Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 13/36] mnt: Tuck mounts under others instead of creating shadow/side mounts Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 14/36] IB/ipoib: Fix deadlock between rmmod and set_mode Greg Kroah-Hartman
2017-03-17 2:24 ` Ben Hutchings
2017-03-13 8:39 ` [PATCH 4.4 15/36] IB/IPoIB: Add destination address when re-queue packet Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 16/36] IB/srp: Avoid that duplicate responses trigger a kernel bug Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 17/36] IB/srp: Fix race conditions related to task management Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 18/36] ktest: Fix child exit code processing Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 19/36] ceph: remove req from unsafe list when unregistering it Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 20/36] target: Fix NULL dereference during LUN lookup + active I/O shutdown Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 21/36] nlm: Ensure callback code also checks that the files match Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 22/36] pwm: pca9685: Fix period change with same duty cycle Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 23/36] xtensa: move parse_tag_fdt out of #ifdef CONFIG_BLK_DEV_INITRD Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 24/36] mac80211: flush delayed work when entering suspend Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 26/36] drm/ast: Fix test for VGA enabled Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 27/36] drm/ast: Call open_key before enable_mmio in POST code Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 28/36] drm/ast: Fix AST2400 POST failure without BMC FW or VBIOS Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 31/36] drm/atomic: fix an error code in mode_fixup() Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 32/36] fakelb: fix schedule while atomic Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 34/36] libceph: use BUG() instead of BUG_ON(1) Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 35/36] fat: fix using uninitialized fields of fat_inode/fsinfo_inode Greg Kroah-Hartman
2017-03-13 8:39 ` [PATCH 4.4 36/36] drivers: hv: Turn off write permission on the hypercall page Greg Kroah-Hartman
2017-03-13 22:36 ` [PATCH 4.4 00/36] 4.4.54-stable review Guenter Roeck
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=20170313083352.664793386@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=alex.popov@linux.com \
--cc=linux-kernel@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 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).