Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eeb2a2ca authored by Duoming Zhou's avatar Duoming Zhou Committed by Greg Kroah-Hartman
Browse files

ALSA: sh: aica: reorder cleanup operations to avoid UAF bugs 



commit 051e0840ffa8ab25554d6b14b62c9ab9e4901457 upstream.

The dreamcastcard->timer could schedule the spu_dma_work and the
spu_dma_work could also arm the dreamcastcard->timer.

When the snd_pcm_substream is closing, the aica_channel will be
deallocated. But it could still be dereferenced in the worker
thread. The reason is that del_timer() will return directly
regardless of whether the timer handler is running or not and
the worker could be rescheduled in the timer handler. As a result,
the UAF bug will happen. The racy situation is shown below:

      (Thread 1)                 |      (Thread 2)
snd_aicapcm_pcm_close()          |
 ...                             |  run_spu_dma() //worker
                                 |    mod_timer()
  flush_work()                   |
  del_timer()                    |  aica_period_elapsed() //timer
  kfree(dreamcastcard->channel)  |    schedule_work()
                                 |  run_spu_dma() //worker
  ...                            |    dreamcastcard->channel-> //USE

In order to mitigate this bug and other possible corner cases,
call mod_timer() conditionally in run_spu_dma(), then implement
PCM sync_stop op to cancel both the timer and worker. The sync_stop
op will be called from PCM core appropriately when needed.

Fixes: 198de43d ("[ALSA] Add ALSA support for the SEGA Dreamcast PCM device")
Suggested-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarDuoming Zhou <duoming@zju.edu.cn>
Message-ID: <20240326094238.95442-1-duoming@zju.edu.cn>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 59044112

sound/sh/aica.c

+14 −3
Original line number Original line Diff line number Diff line
@@ -295,6 +295,7 @@ static void run_spu_dma(struct work_struct *work) 
		dreamcastcard->clicks++;

		dreamcastcard->clicks++;

		if (unlikely(dreamcastcard->clicks >= AICA_PERIOD_NUMBER))

		if (unlikely(dreamcastcard->clicks >= AICA_PERIOD_NUMBER))

			dreamcastcard->clicks %= AICA_PERIOD_NUMBER;

			dreamcastcard->clicks %= AICA_PERIOD_NUMBER;

		if (snd_pcm_running(dreamcastcard->substream))

			mod_timer(&dreamcastcard->timer, jiffies + 1);

			mod_timer(&dreamcastcard->timer, jiffies + 1);

	}

	}

}

}
@@ -307,6 +308,8 @@ static void aica_period_elapsed(struct timer_list *t) 
	/*timer function - so cannot sleep */

	/*timer function - so cannot sleep */

	int play_period;

	int play_period;

	struct snd_pcm_runtime *runtime;

	struct snd_pcm_runtime *runtime;

	if (!snd_pcm_running(substream))

		return;

	runtime = substream->runtime;

	runtime = substream->runtime;

	dreamcastcard = substream->pcm->private_data;

	dreamcastcard = substream->pcm->private_data;

	/* Have we played out an additional period? */

	/* Have we played out an additional period? */
@@ -367,12 +370,19 @@ static int snd_aicapcm_pcm_open(struct snd_pcm_substream 
	return 0;

	return 0;

}

}





static int snd_aicapcm_pcm_sync_stop(struct snd_pcm_substream *substream)

{

	struct snd_card_aica *dreamcastcard = substream->pcm->private_data;



	del_timer_sync(&dreamcastcard->timer);

	cancel_work_sync(&dreamcastcard->spu_dma_work);

	return 0;

}



static int snd_aicapcm_pcm_close(struct snd_pcm_substream

static int snd_aicapcm_pcm_close(struct snd_pcm_substream

				 *substream)

				 *substream)

{

{

	struct snd_card_aica *dreamcastcard = substream->pcm->private_data;

	struct snd_card_aica *dreamcastcard = substream->pcm->private_data;

	flush_work(&(dreamcastcard->spu_dma_work));

	del_timer(&dreamcastcard->timer);

	dreamcastcard->substream = NULL;

	dreamcastcard->substream = NULL;

	kfree(dreamcastcard->channel);

	kfree(dreamcastcard->channel);

	spu_disable();

	spu_disable();
@@ -438,6 +448,7 @@ static const struct snd_pcm_ops snd_aicapcm_playback_ops = { 
	.prepare = snd_aicapcm_pcm_prepare,

	.prepare = snd_aicapcm_pcm_prepare,

	.trigger = snd_aicapcm_pcm_trigger,

	.trigger = snd_aicapcm_pcm_trigger,

	.pointer = snd_aicapcm_pcm_pointer,

	.pointer = snd_aicapcm_pcm_pointer,

	.sync_stop = snd_aicapcm_pcm_sync_stop,

};

};





/* TO DO: set up to handle more than one pcm instance */

/* TO DO: set up to handle more than one pcm instance */