Patch "ALSA: rawmidi: Make snd_rawmidi_transmit() race-free" has been added to the 4.4-stable tree

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

ALSA: rawmidi: Make snd_rawmidi_transmit() race-free

to the 4.4-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-make-snd_rawmidi_transmit-race-free.patch
and it can be found in the queue-4.4 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: Sun, 31 Jan 2016 11:57:41 +0100
Subject: ALSA: rawmidi: Make snd_rawmidi_transmit() race-free

From: Takashi Iwai <[email protected]>

commit 06ab30034ed9c200a570ab13c017bde248ddb2a6 upstream.

A kernel WARNING in snd_rawmidi_transmit_ack() is triggered by syzkaller fuzzer:

  WARNING: CPU: 1 PID: 20739 at sound/core/rawmidi.c:1136
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff82999e2d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
 [<ffffffff81352089>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
 [<ffffffff813522b9>] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
 [<ffffffff84f80bd5>] snd_rawmidi_transmit_ack+0x275/0x400 sound/core/rawmidi.c:1136
 [<ffffffff84fdb3c1>] snd_virmidi_output_trigger+0x4b1/0x5a0 sound/core/seq/seq_virmidi.c:163
 [<     inline     >] snd_rawmidi_output_trigger sound/core/rawmidi.c:150
 [<ffffffff84f87ed9>] snd_rawmidi_kernel_write1+0x549/0x780 sound/core/rawmidi.c:1223
 [<ffffffff84f89fd3>] snd_rawmidi_write+0x543/0xb30 sound/core/rawmidi.c:1273
 [<ffffffff817b0323>] __vfs_write+0x113/0x480 fs/read_write.c:528
 [<ffffffff817b1db7>] vfs_write+0x167/0x4a0 fs/read_write.c:577
 [<     inline     >] SYSC_write fs/read_write.c:624
 [<ffffffff817b50a1>] SyS_write+0x111/0x220 fs/read_write.c:616
 [<ffffffff86336c36>] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185

Also a similar warning is found but in another path:

Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff82be2c0d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
 [<ffffffff81355139>] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482
 [<ffffffff81355369>] warn_slowpath_null+0x29/0x30 kernel/panic.c:515
 [<ffffffff8527e69a>] rawmidi_transmit_ack+0x24a/0x3b0 sound/core/rawmidi.c:1133
 [<ffffffff8527e851>] snd_rawmidi_transmit_ack+0x51/0x80 sound/core/rawmidi.c:1163
 [<ffffffff852d9046>] snd_virmidi_output_trigger+0x2b6/0x570 sound/core/seq/seq_virmidi.c:185
 [<     inline     >] snd_rawmidi_output_trigger sound/core/rawmidi.c:150
 [<ffffffff85285a0b>] snd_rawmidi_kernel_write1+0x4bb/0x760 sound/core/rawmidi.c:1252
 [<ffffffff85287b73>] snd_rawmidi_write+0x543/0xb30 sound/core/rawmidi.c:1302
 [<ffffffff817ba5f3>] __vfs_write+0x113/0x480 fs/read_write.c:528
 [<ffffffff817bc087>] vfs_write+0x167/0x4a0 fs/read_write.c:577
 [<     inline     >] SYSC_write fs/read_write.c:624
 [<ffffffff817bf371>] SyS_write+0x111/0x220 fs/read_write.c:616
 [<ffffffff86660276>] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185

In the former case, the reason is that virmidi has an open code calling snd_rawmidi_transmit_ack() with the value calculated outside
the spinlock. We may use snd_rawmidi_transmit() in a loop just for
consuming the input data, but even there, there is a race between snd_rawmidi_transmit_peek() and snd_rawmidi_tranmit_ack().

Similarly in the latter case, it calls snd_rawmidi_transmit_peek() and snd_rawmidi_tranmit_ack() separately without protection, so they are racy as well.

The patch tries to address these issues by the following ways:
- Introduce the unlocked versions of snd_rawmidi_transmit_peek() and snd_rawmidi_transmit_ack() to be called inside the explicit lock.
- Rewrite snd_rawmidi_transmit() to be race-free (the former case).
- Make the split calls (the latter case) protected in the rawmidi spin lock.

---
 include/sound/rawmidi.h      |    4 +
 sound/core/rawmidi.c         |   98 +++++++++++++++++++++++++++++++------------
 sound/core/seq/seq_virmidi.c |   17 +++++--
 3 files changed, 88 insertions(+), 31 deletions(-)

@@ -167,6 +167,10 @@ int snd_rawmidi_transmit_peek(struct snd
 int snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream, int count);
 int snd_rawmidi_transmit(struct snd_rawmidi_substream *substream,
 			 unsigned char *buffer, int count);
+int __snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream,
+			      unsigned char *buffer, int count);
+int __snd_rawmidi_transmit_ack(struct snd_rawmidi_substream *substream,
+			       int count);
 
 /* main midi functions */
 

@@ -155,21 +155,26 @@ static void snd_virmidi_output_trigger(s
 	struct snd_virmidi *vmidi = substream->runtime->private_data;
 	int count, res;
 	unsigned char buf[32], *pbuf;
+	unsigned long flags;
 
 	if (up) {
 		vmidi->trigger = 1;
 		if (vmidi->seq_mode == SNDRV_VIRMIDI_SEQ_DISPATCH &&
 		    !(vmidi->rdev->flags & SNDRV_VIRMIDI_SUBSCRIBE)) {
-			snd_rawmidi_transmit_ack(substream, substream->runtime->buffer_size - substream->runtime->avail);
-			return;		/* ignored */
+			while (snd_rawmidi_transmit(substream, buf,
+						    sizeof(buf)) > 0) {
+				/* ignored */
+			}
+			return;
 		}
 		if (vmidi->event.type != SNDRV_SEQ_EVENT_NONE) {
 			if (snd_seq_kernel_client_dispatch(vmidi->client, &vmidi->event, in_atomic(), 0) < 0)
 				return;
 			vmidi->event.type = SNDRV_SEQ_EVENT_NONE;
 		}
+		spin_lock_irqsave(&substream->runtime->lock, flags);
 		while (1) {
-			count = snd_rawmidi_transmit_peek(substream, buf, sizeof(buf));
+			count = __snd_rawmidi_transmit_peek(substream, buf, sizeof(buf));
 			if (count <= 0)
 				break;
 			pbuf = buf;

@@ -179,16 +184,18 @@ static void snd_virmidi_output_trigger(s
 					snd_midi_event_reset_encode(vmidi->parser);
 					continue;
 				}
-				snd_rawmidi_transmit_ack(substream, res);
+				__snd_rawmidi_transmit_ack(substream, res);
 				pbuf += res;
 				count -= res;
 				if (vmidi->event.type != SNDRV_SEQ_EVENT_NONE) {
 					if (snd_seq_kernel_client_dispatch(vmidi->client, &vmidi->event, in_atomic(), 0) < 0)
-						return;
+						goto out;
 					vmidi->event.type = SNDRV_SEQ_EVENT_NONE;
 				}
 			}
 		}
+	out:
+		spin_unlock_irqrestore(&substream->runtime->lock, flags);
 	} else {
 		vmidi->trigger = 0;
 	}

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

queue-4.4/alsa-seq-fix-race-at-closing-in-virmidi-driver.patch queue-4.4/alsa-hda-fix-bad-dereference-of-jack-object.patch queue-4.4/alsa-usb-audio-add-quirk-for-microsoft-lifecam-hd-6000.patch queue-4.4/alsa-rawmidi-remove-kernel-warning-for-null-user-space-buffer-check.patch queue-4.4/alsa-usb-audio-fix-oppo-ha-1-vendor-id.patch queue-4.4/alsa-timer-fix-race-at-concurrent-reads.patch queue-4.4/alsa-hda-realtek-support-dell-headset-mode-for-alc225.patch queue-4.4/alsa-hda-fix-static-checker-warning-in-patch_hdmi.c.patch queue-4.4/alsa-seq-fix-lockdep-warnings-due-to-double-mutex-locks.patch queue-4.4/alsa-usb-audio-fix-teac-ud-501-ud-503-nt-503-usb-delay.patch queue-4.4/alsa-timer-fix-wrong-instance-passed-to-slave-callbacks.patch queue-4.4/alsa-hda-disable-dynamic-clock-gating-on-broxton-before-reset.patch queue-4.4/alsa-hda-realtek-support-headset-mode-for-alc225.patch queue-4.4/alsa-hda-implement-loopback-control-switch-for-realtek-and-other-codecs.patch queue-4.4/alsa-hda-realtek-new-codec-support-of-alc225.patch queue-4.4/alsa-seq-degrade-the-error-message-for-too-many-opens.patch queue-4.4/alsa-compress-disable-get_codec_caps-ioctl-for-some-architectures.patch queue-4.4/alsa-rawmidi-make-snd_rawmidi_transmit-race-free.patch queue-4.4/alsa-hda-fix-speaker-output-from-vaio-aio-machines.patch queue-4.4/alsa-bebob-use-a-signed-return-type-for-get_formation_index.patch queue-4.4/alsa-add-missing-dependency-on-config_snd_timer.patch queue-4.4/alsa-dummy-implement-timer-backend-switching-more-safely.patch queue-4.4/alsa-dummy-disable-switching-timer-backend-via-sysfs.patch queue-4.4/alsa-seq-fix-incorrect-sanity-check-at-snd_seq_oss_synth_cleanup.patch queue-4.4/alsa-seq-fix-yet-another-races-among-alsa-timer-accesses.patch queue-4.4/alsa-usb-audio-avoid-freeing-umidi-object-twice.patch queue-4.4/revert-alsa-hda-fix-noise-on-gigabyte-z170x-mobo.patch queue-4.4/alsa-hda-add-fixup-for-mac-mini-7-1-model.patch queue-4.4/alsa-usb-audio-add-native-dsd-support-for-ps-audio-nuwave-dac.patch queue-4.4/alsa-timer-fix-leftover-link-at-closing.patch queue-4.4/alsa-rawmidi-fix-race-at-copying-updating-the-position.patch queue-4.4/alsa-pcm-fix-potential-deadlock-in-oss-emulation.patch queue-4.4/alsa-timer-fix-link-corruption-due-to-double-start-or-stop.patch