[PATCH 4/6] ALSA: timer: copy queue entries to local buffer before copy_to_user

[Omer Cohen <nevergfx1@gmail.com>]

snd_timer_user_read() dequeues an entry from the timer event queue
under qlock, then drops the spinlock and calls copy_to_user() directly
from the queue slot (tread = &tu->tqueue[qhead]).

While the lock is dropped, timer interrupt callbacks
(snd_timer_user_tinterrupt) can write new entries to the queue.  If
enough events arrive to wrap qtail back to the dequeued slot, the
callback overwrites data that copy_to_user() is still reading.

This is the same class of bug fixed for the non-tread queue path in
CVE-2017-1000380, but the tread queue (tu->tqueue) and the NONE
format queue (tu->queue) were left with the same pattern.

Copy dequeued entries to local stack variables under the spinlock
before dropping it, then use the local copies for copy_to_user().

Fixes: d11662f4f798 ("ALSA: timer: Fix race between read and ioctl")
Cc: stable@vger.kernel.org
Reported-by: Omer Cohen <nevergfx1@gmail.com>
Signed-off-by: Omer Cohen <nevergfx1@gmail.com>
---
 sound/core/timer.c | 19 +++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
index 51c6ac4df9f4..XXXXXXXXXXXX 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -2390,6 +2390,7 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
-	struct snd_timer_tread64 *tread;
+	struct snd_timer_tread64 tread_local;
 	struct snd_timer_tread32 tread32;
+	struct snd_timer_read read_local;
 	struct snd_timer_user *tu;
 	long result = 0, unit;
 	int tread_format;
 	int qhead;
@@ -2452,23 +2454,23 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
 		qhead = tu->qhead++;
 		tu->qhead %= tu->queue_size;
 		tu->qused--;
+		tread_local = tu->tqueue[qhead];
+		read_local = tu->queue[qhead];
 		spin_unlock_irq(&tu->qlock);

-		tread = &tu->tqueue[qhead];
-
 		switch (tread_format) {
 		case TREAD_FORMAT_TIME64:
-			if (copy_to_user(buffer, tread,
+			if (copy_to_user(buffer, &tread_local,
 					 sizeof(struct snd_timer_tread64)))
 				err = -EFAULT;
 			break;
 		case TREAD_FORMAT_TIME32:
 			memset(&tread32, 0, sizeof(tread32));
 			tread32 = (struct snd_timer_tread32) {
-				.event = tread->event,
-				.tstamp_sec = tread->tstamp_sec,
-				.tstamp_nsec = tread->tstamp_nsec,
-				.val = tread->val,
+				.event = tread_local.event,
+				.tstamp_sec = tread_local.tstamp_sec,
+				.tstamp_nsec = tread_local.tstamp_nsec,
+				.val = tread_local.val,
 			};

 			if (copy_to_user(buffer, &tread32, sizeof(tread32)))
@@ -2470,6 +2474,6 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
 			break;
 		case TREAD_FORMAT_NONE:
-			if (copy_to_user(buffer, &tu->queue[qhead],
+			if (copy_to_user(buffer, &read_local,
 					 sizeof(struct snd_timer_read)))
 				err = -EFAULT;
 			break;
--
2.43.0