Patch "tty: n_hdlc: get rid of racy n_hdlc.tbuf" has been added to the 4.9-stable tree
Subsystems (2)
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.9-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.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From
82f2341c94d270421f383641b7cd670e474db56bMon Sep 17 00:00:00 2001
From: Alexander Popov <[email protected]>
Date: Tue, 28 Feb 2017 19:54:40 +0300
Subject: tty: n_hdlc: get rid of racy n_hdlc.tbuf
From: Alexander Popov <[email protected]>
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.
--- drivers/tty/n_hdlc.c | 132 ++++++++++++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 63 deletions(-)
drivers/tty/n_hdlc.c +69 -63
@@ -114,7 +114,7 @@#define DEFAULT_TX_BUF_COUNT 3struct 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_sint 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(strupoll_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 [email protected] are
queue-4.9/tty-n_hdlc-get-rid-of-racy-n_hdlc.tbuf.patch