Linux kernel TTY NULL Pointer Dereference Race Condition

This bug is a result of twitter. By that, I mean that I saw this in spender’s twitter today. According to the bug report, this vulnerability was discovered by Kyle Bader and it affects 2.6.31.5 and probably other releases too.
Here is the susceptible code as seen in drivers/char/n_tty.c file of 2.6.31.5 version of the Linux kernel…

/**
 *	n_tty_close		-	close the ldisc for this tty
 *	@tty: device
 *
 *	Called from the terminal layer when this line discipline is
 *	being shut down, either because of a close or becsuse of a
 *	discipline change. The function will not be called while other
 *	ldisc methods are in progress.
 */

static void n_tty_close(struct tty_struct *tty)
{
	n_tty_flush_buffer(tty);
	if (tty->read_buf) {
		kfree(tty->read_buf);
		tty->read_buf = NULL;
	}
	if (tty->echo_buf) {
		kfree(tty->echo_buf);
		tty->echo_buf = NULL;
	}
}

The use of this routine is quite clear from the provided comments. So, its argument is a TTY passed to it as a pointer to a ‘tty_struct’ structure and as you can see, it initially invokes n_tty_flush_buffer() and then, frees the allocated buffers ‘read_buf’ and ‘echo_buf’ and sets them to NULL. Let’s have a look at the first function being called in there…

/**
 *	n_tty_flush_buffer	-	clean input queue
 *	@tty:	terminal device
 *
 *	Flush the input buffer. Called when the line discipline is
 *	being closed, when the tty layer wants the buffer flushed (eg
 *	at hangup) or when the N_TTY line discipline internally has to
 *	clean the pending queue (for example some signals).
 *
 *	Locking: ctrl_lock, read_lock.
 */

static void n_tty_flush_buffer(struct tty_struct *tty)
{
	unsigned long flags;
	/* clear everything and unthrottle the driver */
	reset_buffer_flags(tty);

	if (!tty->link)
		return;

	spin_lock_irqsave(&tty->ctrl_lock, flags);
	if (tty->link->packet) {
		tty->ctrl_status |= TIOCPKT_FLUSHREAD;
		wake_up_interruptible(&tty->link->read_wait);
	}
	spin_unlock_irqrestore(&tty->ctrl_lock, flags);
}

The important part here as you probably have read in the original bug report, is that the only locks being held is a spin lock inside n_tty_flush_buffer() that you can see above. Since there are no other locks during this process, it could result in a race condition. Specifically, n_tty_close() doesn’t lock during the de-allocation of the buffers and during the kfree() of the pointers, if the TTY receives a character the code execution could reach the following function:

static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
{
	unsigned long flags;
	int parmrk;

	if (tty->raw) {
		put_tty_queue(c, tty);
		return;
	}
   ...
	put_tty_queue(c, tty);
}

The result would be:

static void put_tty_queue(unsigned char c, struct tty_struct *tty)
{
	unsigned long flags;
	/*
	 *	The problem of stomping on the buffers ends here.
	 *	Why didn't anyone see this one coming? --AJK
	*/
	spin_lock_irqsave(&tty->read_lock, flags);
	put_tty_queue_nolock(c, tty);
	spin_unlock_irqrestore(&tty->read_lock, flags);
}

Which locks the TTY and invokes the equivalent no-locking routine that will simply execute the following…

static void put_tty_queue_nolock(unsigned char c, struct tty_struct *tty)
{
	if (tty->read_cnt < N_TTY_BUF_SIZE) {
		tty->read_buf[tty->read_head] = c;
		tty->read_head = (tty->read_head + 1) & (N_TTY_BUF_SIZE-1);
		tty->read_cnt++;
	}
}

Since ‘tty->read_buf’ could have been freed and set to NULL in n_tty_close() by that time, the assignment of the new character ‘c’ would result in an attempt to access and perform an arbitrary write to a NULL pointer. Consequently, this will lead to a NULL pointer dereference.
The most weird comment in the bugzilla, was that of “Sock Puppet” who says that grsecurity patch contains a bugfix. Correct me if I’m wrong, but I didn’t saw anything for that specific bug… Unless, this is just sarcasm and its intention is to suggest UDEREF as a solution but it’s still not a bugfix…