ALSA: sh: aica: reorder cleanup operations to avoid UAF bugs
[ Upstream commit 051e0840ffa8ab25554d6b14b62c9ab9e4901457 ]
Fix CVE: CVE-2024-26654
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: 198de43d75
("[ALSA] Add ALSA support for the SEGA Dreamcast PCM device")
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
Message-ID: <20240326094238.95442-1-duoming@zju.edu.cn>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Jianping Liu <frankjpliu@tencent.com>
This commit is contained in:
parent
5f1a749c9b
commit
323f474585
|
@ -279,6 +279,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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -291,6 +292,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? */
|
||||||
|
@ -351,12 +354,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();
|
||||||
|
@ -422,6 +432,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 */
|
||||||
|
|
Loading…
Reference in New Issue