Patch "ALSA: rawmidi: Fix race at copying & updating the position" has been added to the 3.14-stable tree

This is a note to let you know that I've just added the patch titled

ALSA: rawmidi: Fix race at copying & updating the position

to the 3.14-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:
alsa-rawmidi-fix-race-at-copying-updating-the-position.patch
and it can be found in the queue-3.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: Takashi Iwai <[email protected]>
Date: Wed, 3 Feb 2016 14:41:22 +0100
Subject: ALSA: rawmidi: Fix race at copying & updating the position

From: Takashi Iwai <[email protected]>

commit 81f577542af15640cbcb6ef68baa4caa610cbbfc upstream.

The rawmidi read and write functions manage runtime stream status such as runtime->appl_ptr and runtime->avail. These point where to copy the new data and how many bytes have been copied (or to be read). The problem is that rawmidi read/write call copy_from_user() or copy_to_user(), and the runtime spinlock is temporarily unlocked and relocked while copying user-space. Since the current code advances and updates the runtime status after the spin unlock/relock, the copy and the update may be asynchronous, and eventually runtime->avail might go to a negative value when many concurrent accesses are done. This may lead to memory corruption in the end.

For fixing this race, in this patch, the status update code is performed in the same lock before the temporary unlock. Also, the spinlock is now taken more widely in snd_rawmidi_kernel_read1() for protecting more properly during the whole operation.

---
 sound/core/rawmidi.c |   34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

@@ -934,31 +934,36 @@ static long snd_rawmidi_kernel_read1(str
 	unsigned long flags;
 	long result = 0, count1;
 	struct snd_rawmidi_runtime *runtime = substream->runtime;
+	unsigned long appl_ptr;
 
+	spin_lock_irqsave(&runtime->lock, flags);
 	while (count > 0 && runtime->avail) {
 		count1 = runtime->buffer_size - runtime->appl_ptr;
 		if (count1 > count)
 			count1 = count;
-		spin_lock_irqsave(&runtime->lock, flags);
 		if (count1 > (int)runtime->avail)
 			count1 = runtime->avail;
+
+		/* update runtime->appl_ptr before unlocking for userbuf */
+		appl_ptr = runtime->appl_ptr;
+		runtime->appl_ptr += count1;
+		runtime->appl_ptr %= runtime->buffer_size;
+		runtime->avail -= count1;
+
 		if (kernelbuf)
-			memcpy(kernelbuf + result, runtime->buffer + runtime->appl_ptr, count1);
+			memcpy(kernelbuf + result, runtime->buffer + appl_ptr, count1);
 		if (userbuf) {
 			spin_unlock_irqrestore(&runtime->lock, flags);
 			if (copy_to_user(userbuf + result,
-					 runtime->buffer + runtime->appl_ptr, count1)) {
+					 runtime->buffer + appl_ptr, count1)) {
 				return result > 0 ? result : -EFAULT;
 			}
 			spin_lock_irqsave(&runtime->lock, flags);
 		}
-		runtime->appl_ptr += count1;
-		runtime->appl_ptr %= runtime->buffer_size;
-		runtime->avail -= count1;
-		spin_unlock_irqrestore(&runtime->lock, flags);
 		result += count1;
 		count -= count1;
 	}
+	spin_unlock_irqrestore(&runtime->lock, flags);
 	return result;
 }
 

@@ -1161,6 +1166,7 @@ static long snd_rawmidi_kernel_write1(st
 	unsigned long flags;
 	long count1, result;
 	struct snd_rawmidi_runtime *runtime = substream->runtime;
+	unsigned long appl_ptr;
 
 	if (!kernelbuf && !userbuf)
 		return -EINVAL;

@@ -1181,12 +1187,19 @@ static long snd_rawmidi_kernel_write1(st
 			count1 = count;
 		if (count1 > (long)runtime->avail)
 			count1 = runtime->avail;
+
+		/* update runtime->appl_ptr before unlocking for userbuf */
+		appl_ptr = runtime->appl_ptr;
+		runtime->appl_ptr += count1;
+		runtime->appl_ptr %= runtime->buffer_size;
+		runtime->avail -= count1;
+
 		if (kernelbuf)
-			memcpy(runtime->buffer + runtime->appl_ptr,
+			memcpy(runtime->buffer + appl_ptr,
 			       kernelbuf + result, count1);
 		else if (userbuf) {
 			spin_unlock_irqrestore(&runtime->lock, flags);
-			if (copy_from_user(runtime->buffer + runtime->appl_ptr,
+			if (copy_from_user(runtime->buffer + appl_ptr,
 					   userbuf + result, count1)) {
 				spin_lock_irqsave(&runtime->lock, flags);
 				result = result > 0 ? result : -EFAULT;

@@ -1194,9 +1207,6 @@ static long snd_rawmidi_kernel_write1(st
 			}
 			spin_lock_irqsave(&runtime->lock, flags);
 		}
-		runtime->appl_ptr += count1;
-		runtime->appl_ptr %= runtime->buffer_size;
-		runtime->avail -= count1;
 		result += count1;
 		count -= count1;
 	}

Patches currently in stable-queue which might be from [email protected] are

queue-3.14/alsa-seq-fix-race-at-closing-in-virmidi-driver.patch queue-3.14/alsa-rawmidi-remove-kernel-warning-for-null-user-space-buffer-check.patch queue-3.14/alsa-hda-fix-static-checker-warning-in-patch_hdmi.c.patch queue-3.14/alsa-seq-fix-lockdep-warnings-due-to-double-mutex-locks.patch queue-3.14/alsa-usb-audio-fix-teac-ud-501-ud-503-nt-503-usb-delay.patch queue-3.14/alsa-timer-fix-wrong-instance-passed-to-slave-callbacks.patch queue-3.14/alsa-compress-disable-get_codec_caps-ioctl-for-some-architectures.patch queue-3.14/alsa-hda-fix-speaker-output-from-vaio-aio-machines.patch queue-3.14/alsa-dummy-implement-timer-backend-switching-more-safely.patch queue-3.14/alsa-dummy-disable-switching-timer-backend-via-sysfs.patch queue-3.14/alsa-seq-fix-incorrect-sanity-check-at-snd_seq_oss_synth_cleanup.patch queue-3.14/alsa-seq-fix-yet-another-races-among-alsa-timer-accesses.patch queue-3.14/alsa-usb-audio-avoid-freeing-umidi-object-twice.patch queue-3.14/alsa-hda-add-fixup-for-mac-mini-7-1-model.patch queue-3.14/alsa-timer-fix-leftover-link-at-closing.patch queue-3.14/alsa-rawmidi-fix-race-at-copying-updating-the-position.patch queue-3.14/alsa-pcm-fix-potential-deadlock-in-oss-emulation.patch queue-3.14/alsa-timer-fix-link-corruption-due-to-double-start-or-stop.patch