Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 82f2341c authored by Alexander Popov's avatar Alexander Popov Committed by Greg Kroah-Hartman
Browse files

tty: n_hdlc: get rid of racy n_hdlc.tbuf 



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 be10eb75
("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: default avatarAlexander Popov <alex.popov@linux.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent c1ae3cfa

drivers/tty/n_hdlc.c

+69 −63
Original line number Diff line number Diff line
@@ -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_struct *tty) 
{

	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 *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_hdlc *n_hdlc, struct tty_struct *tty) 
	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);

		

	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_hdlc *n_hdlc, struct tty_struct *tty) 


		/* 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 */
@@ -436,9 +421,6 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty) 
			/* 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);
@@ -449,9 +431,11 @@ static void n_hdlc_send_frames(struct n_hdlc *n_hdlc, struct tty_struct *tty) 
				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,6 +733,7 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file, 
	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",
@@ -763,8 +748,10 @@ static int n_hdlc_tty_ioctl(struct tty_struct *tty, struct file *file, 
		/* 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_struct *tty, struct file *file, 
		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(struct tty_struct *tty, struct file *filp, 
		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;
@@ -857,6 +846,11 @@ static struct n_hdlc *n_hdlc_alloc(void) 
	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);
@@ -883,54 +877,66 @@ 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_lock_irqsave(&buf_list->spinlock, flags);



	spin_unlock_irqrestore(&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 =