CVE-2017-9985: Out-of-bounds Read (CWE-125)
value: The snd_msndmidi_input_read function in sound/isa/msnd/msnd_midi.c in the Linux kernel through 4.11.7 allows local users to cause a denial of service (over-boundary access) or possibly have unspecified other impact by changing the value of a message queue head pointer between two kernel reads of that value, aka a "double fetch" vulnerability.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.8
Published: 2017-06-28T06:29Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: sound/isa/msnd/msnd_midi.c, sound/isa/msnd/msnd_pinnacle.c
Commit date: 2017-07-07 15:34:57 +0200
Commit message: ALSA: msnd: Optimize / harden DSP and MIDI loops

The ISA msnd drivers have loops fetching the ring-buffer head, tail
and size values inside the loops.  Such codes are inefficient and
fragile.

This patch optimizes it, and also adds the sanity check to avoid the
endless loops.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196131
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=196133
Signed-off-by: Takashi Iwai <tiwai@suse.de>

Diff: diff --git a/sound/isa/msnd/msnd_midi.c b/sound/isa/msnd/msnd_midi.c
index 912b5a9ccbab..013d8d1170fe 100644
--- a/sound/isa/msnd/msnd_midi.c
+++ b/sound/isa/msnd/msnd_midi.c
@@ -118,28 +118,28 @@ static void snd_msndmidi_input_trigger(struct snd_rawmidi_substream *substream,
 void snd_msndmidi_input_read(void *mpuv)
 {
 	unsigned long flags;
 	struct snd_msndmidi *mpu = mpuv;
 	void *pwMIDQData = mpu->dev->mappedbase + MIDQ_DATA_BUFF;
+	u16 head, tail, size;
 
 	spin_lock_irqsave(&mpu->input_lock, flags);
-	while (readw(mpu->dev->MIDQ + JQS_wTail) !=
-	       readw(mpu->dev->MIDQ + JQS_wHead)) {
-		u16 wTmp, val;
-		val = readw(pwMIDQData + 2 * readw(mpu->dev->MIDQ + JQS_wHead));
-
-			if (test_bit(MSNDMIDI_MODE_BIT_INPUT_TRIGGER,
-				     &mpu->mode))
-				snd_rawmidi_receive(mpu->substream_input,
-						    (unsigned char *)&val, 1);
-
-		wTmp = readw(mpu->dev->MIDQ + JQS_wHead) + 1;
-		if (wTmp > readw(mpu->dev->MIDQ + JQS_wSize))
-			writew(0,  mpu->dev->MIDQ + JQS_wHead);
-		else
-			writew(wTmp,  mpu->dev->MIDQ + JQS_wHead);
+	head = readw(mpu->dev->MIDQ + JQS_wHead);
+	tail = readw(mpu->dev->MIDQ + JQS_wTail);
+	size = readw(mpu->dev->MIDQ + JQS_wSize);
+	if (head > size || tail > size)
+		goto out;
+	while (head != tail) {
+		unsigned char val = readw(pwMIDQData + 2 * head);
+
+		if (test_bit(MSNDMIDI_MODE_BIT_INPUT_TRIGGER, &mpu->mode))
+			snd_rawmidi_receive(mpu->substream_input, &val, 1);
+		if (++head > size)
+			head = 0;
+		writew(head, mpu->dev->MIDQ + JQS_wHead);
 	}
+ out:
 	spin_unlock_irqrestore(&mpu->input_lock, flags);
 }
 EXPORT_SYMBOL(snd_msndmidi_input_read);
 
 static const struct snd_rawmidi_ops snd_msndmidi_input = {
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c
index ad4897337df5..fc4fb1904aef 100644
--- a/sound/isa/msnd/msnd_pinnacle.c
+++ b/sound/isa/msnd/msnd_pinnacle.c
@@ -168,27 +168,28 @@ static void snd_msnd_eval_dsp_msg(struct snd_msnd *chip, u16 wMessage)
 
 static irqreturn_t snd_msnd_interrupt(int irq, void *dev_id)
 {
 	struct snd_msnd *chip = dev_id;
 	void *pwDSPQData = chip->mappedbase + DSPQ_DATA_BUFF;
+	u16 head, tail, size;
 
 	/* Send ack to DSP */
 	/* inb(chip->io + HP_RXL); */
 
 	/* Evaluate queued DSP messages */
-	while (readw(chip->DSPQ + JQS_wTail) != readw(chip->DSPQ + JQS_wHead)) {
-		u16 wTmp;
-
-		snd_msnd_eval_dsp_msg(chip,
-			readw(pwDSPQData + 2 * readw(chip->DSPQ + JQS_wHead)));
-
-		wTmp = readw(chip->DSPQ + JQS_wHead) + 1;
-		if (wTmp > readw(chip->DSPQ + JQS_wSize))
-			writew(0, chip->DSPQ + JQS_wHead);
-		else
-			writew(wTmp, chip->DSPQ + JQS_wHead);
+	head = readw(chip->DSPQ + JQS_wHead);
+	tail = readw(chip->DSPQ + JQS_wTail);
+	size = readw(chip->DSPQ + JQS_wSize);
+	if (head > size || tail > size)
+		goto out;
+	while (head != tail) {
+		snd_msnd_eval_dsp_msg(chip, readw(pwDSPQData + 2 * head));
+		if (++head > size)
+			head = 0;
+		writew(head, chip->DSPQ + JQS_wHead);
 	}
+ out:
 	/* Send ack to DSP */
 	inb(chip->io + HP_RXL);
 	return IRQ_HANDLED;
 }
 

Vulnerable code: 	while (readw(mpu->dev->MIDQ + JQS_wTail) !=
	       readw(mpu->dev->MIDQ + JQS_wHead)) {
		u16 wTmp, val;
		val = readw(pwMIDQData + 2 * readw(mpu->dev->MIDQ + JQS_wHead));

			if (test_bit(MSNDMIDI_MODE_BIT_INPUT_TRIGGER,
				     &mpu->mode))
				snd_rawmidi_receive(mpu->substream_input,
						    (unsigned char *)&val, 1);

		wTmp = readw(mpu->dev->MIDQ + JQS_wHead) + 1;
		if (wTmp > readw(mpu->dev->MIDQ + JQS_wSize))
			writew(0,  mpu->dev->MIDQ + JQS_wHead);
		else
			writew(wTmp,  mpu->dev->MIDQ + JQS_wHead);
	while (readw(chip->DSPQ + JQS_wTail) != readw(chip->DSPQ + JQS_wHead)) {
		u16 wTmp;

		snd_msnd_eval_dsp_msg(chip,
			readw(pwDSPQData + 2 * readw(chip->DSPQ + JQS_wHead)));

		wTmp = readw(chip->DSPQ + JQS_wHead) + 1;
		if (wTmp > readw(chip->DSPQ + JQS_wSize))
			writew(0, chip->DSPQ + JQS_wHead);
		else
			writew(wTmp, chip->DSPQ + JQS_wHead);
Fixed code: 	u16 head, tail, size;
	head = readw(mpu->dev->MIDQ + JQS_wHead);
	tail = readw(mpu->dev->MIDQ + JQS_wTail);
	size = readw(mpu->dev->MIDQ + JQS_wSize);
	if (head > size || tail > size)
		goto out;
	while (head != tail) {
		unsigned char val = readw(pwMIDQData + 2 * head);

		if (test_bit(MSNDMIDI_MODE_BIT_INPUT_TRIGGER, &mpu->mode))
			snd_rawmidi_receive(mpu->substream_input, &val, 1);
		if (++head > size)
			head = 0;
		writew(head, mpu->dev->MIDQ + JQS_wHead);
 out:
	u16 head, tail, size;
	head = readw(chip->DSPQ + JQS_wHead);
	tail = readw(chip->DSPQ + JQS_wTail);
	size = readw(chip->DSPQ + JQS_wSize);
	if (head > size || tail > size)
		goto out;
	while (head != tail) {
		snd_msnd_eval_dsp_msg(chip, readw(pwDSPQData + 2 * head));
		if (++head > size)
			head = 0;
		writew(head, chip->DSPQ + JQS_wHead);
 out: