diff mbox series

ALSA: msnd: Optimize / harden DSP and MIDI loops

Message ID 1532426211-24585-2-git-send-email-paolo.pisati@canonical.com
State New
Headers show
Series ALSA: msnd: Optimize / harden DSP and MIDI loops | expand

Commit Message

Paolo Pisati July 24, 2018, 9:56 a.m. UTC 
From: Takashi Iwai <tiwai@suse.de>

CVE-2017-9984

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>
(cherry picked from commit 20e2b791796bd68816fa115f12be5320de2b8021)
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 sound/isa/msnd/msnd_midi.c     | 30 +++++++++++++++---------------
 sound/isa/msnd/msnd_pinnacle.c | 23 ++++++++++++-----------
 2 files changed, 27 insertions(+), 26 deletions(-)

Comments

Stefan Bader July 24, 2018, 12:31 p.m. UTC | #1 
On 24.07.2018 11:56, Paolo Pisati wrote:
> From: Takashi Iwai <tiwai@suse.de>
> 
> CVE-2017-9984
> 
> 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>
> (cherry picked from commit 20e2b791796bd68816fa115f12be5320de2b8021)
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  sound/isa/msnd/msnd_midi.c     | 30 +++++++++++++++---------------
>  sound/isa/msnd/msnd_pinnacle.c | 23 ++++++++++++-----------
>  2 files changed, 27 insertions(+), 26 deletions(-)
> 
> diff --git a/sound/isa/msnd/msnd_midi.c b/sound/isa/msnd/msnd_midi.c
> index ffc67fd..58e59cd 100644
> --- a/sound/isa/msnd/msnd_midi.c
> +++ b/sound/isa/msnd/msnd_midi.c
> @@ -120,24 +120,24 @@ 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);
> diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c
> index 0a90bd6..7fd91cd 100644
> --- a/sound/isa/msnd/msnd_pinnacle.c
> +++ b/sound/isa/msnd/msnd_pinnacle.c
> @@ -170,23 +170,24 @@ 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;
>
diff mbox series

Patch

diff --git a/sound/isa/msnd/msnd_midi.c b/sound/isa/msnd/msnd_midi.c
index ffc67fd..58e59cd 100644
--- a/sound/isa/msnd/msnd_midi.c
+++ b/sound/isa/msnd/msnd_midi.c
@@ -120,24 +120,24 @@  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);
diff --git a/sound/isa/msnd/msnd_pinnacle.c b/sound/isa/msnd/msnd_pinnacle.c
index 0a90bd6..7fd91cd 100644
--- a/sound/isa/msnd/msnd_pinnacle.c
+++ b/sound/isa/msnd/msnd_pinnacle.c
@@ -170,23 +170,24 @@  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;