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

Commit 9cb6c40a authored by Takashi Iwai's avatar Takashi Iwai Committed by Greg Kroah-Hartman
Browse files

ALSA: pcm: Fix races among concurrent hw_params and hw_free calls 



commit 92ee3c60ec9fe64404dc035e7c41277d74aa26cb upstream.

Currently we have neither proper check nor protection against the
concurrent calls of PCM hw_params and hw_free ioctls, which may result
in a UAF.  Since the existing PCM stream lock can't be used for
protecting the whole ioctl operations, we need a new mutex to protect
those racy calls.

This patch introduced a new mutex, runtime->buffer_mutex, and applies
it to both hw_params and hw_free ioctl code paths.  Along with it, the
both functions are slightly modified (the mmap_count check is moved
into the state-check block) for code simplicity.

Reported-by: default avatarHu Jiahui <kirin.say@gmail.com>
Cc: <stable@vger.kernel.org>
Reviewed-by: default avatarJaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20220322170720.3529-2-tiwai@suse.de


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
[OP: backport to 4.19: adjusted context]
Signed-off-by: default avatarOvidiu Panait <ovidiu.panait@windriver.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 0779b2ef

include/sound/pcm.h

+1 −0
Original line number Diff line number Diff line
@@ -404,6 +404,7 @@ struct snd_pcm_runtime { 
	wait_queue_head_t sleep;	/* poll sleep */

	wait_queue_head_t tsleep;	/* transfer sleep */

	struct fasync_struct *fasync;

	struct mutex buffer_mutex;	/* protect for buffer changes */



	/* -- private section -- */

	void *private_data;

sound/core/pcm.c

+2 −0
Original line number Diff line number Diff line
@@ -1031,6 +1031,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream, 
	init_waitqueue_head(&runtime->tsleep);



	runtime->status->state = SNDRV_PCM_STATE_OPEN;

	mutex_init(&runtime->buffer_mutex);



	substream->runtime = runtime;

	substream->private_data = pcm->private_data;
@@ -1062,6 +1063,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) 
	substream->runtime = NULL;

	if (substream->timer)

		spin_unlock_irq(&substream->timer->lock);

	mutex_destroy(&runtime->buffer_mutex);

	kfree(runtime);

	put_pid(substream->pid);

	substream->pid = NULL;

sound/core/pcm_native.c

+36 −19
Original line number Diff line number Diff line
@@ -666,33 +666,40 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm, 
	return 0;

}



#if IS_ENABLED(CONFIG_SND_PCM_OSS)

#define is_oss_stream(substream)	((substream)->oss.oss)

#else

#define is_oss_stream(substream)	false

#endif



static int snd_pcm_hw_params(struct snd_pcm_substream *substream,

			     struct snd_pcm_hw_params *params)

{

	struct snd_pcm_runtime *runtime;

	int err, usecs;

	int err = 0, usecs;

	unsigned int bits;

	snd_pcm_uframes_t frames;



	if (PCM_RUNTIME_CHECK(substream))

		return -ENXIO;

	runtime = substream->runtime;

	mutex_lock(&runtime->buffer_mutex);

	snd_pcm_stream_lock_irq(substream);

	switch (runtime->status->state) {

	case SNDRV_PCM_STATE_OPEN:

	case SNDRV_PCM_STATE_SETUP:

	case SNDRV_PCM_STATE_PREPARED:

		if (!is_oss_stream(substream) &&

		    atomic_read(&substream->mmap_count))

			err = -EBADFD;

		break;

	default:

		snd_pcm_stream_unlock_irq(substream);

		return -EBADFD;

		err = -EBADFD;

		break;

	}

	snd_pcm_stream_unlock_irq(substream);

#if IS_ENABLED(CONFIG_SND_PCM_OSS)

	if (!substream->oss.oss)

#endif

		if (atomic_read(&substream->mmap_count))

			return -EBADFD;

	if (err)

		goto unlock;



	params->rmask = ~0U;

	err = snd_pcm_hw_refine(substream, params);
@@ -769,14 +776,19 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, 
	if ((usecs = period_to_usecs(runtime)) >= 0)

		pm_qos_add_request(&substream->latency_pm_qos_req,

				   PM_QOS_CPU_DMA_LATENCY, usecs);

	return 0;

	err = 0;

 _error:

	if (err) {

		/* hardware might be unusable from this time,

	   so we force application to retry to set

	   the correct hardware parameter settings */

		 * so we force application to retry to set

		 * the correct hardware parameter settings

		 */

		snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN);

		if (substream->ops->hw_free != NULL)

			substream->ops->hw_free(substream);

	}

 unlock:

	mutex_unlock(&runtime->buffer_mutex);

	return err;

}
@@ -809,22 +821,27 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream) 
	if (PCM_RUNTIME_CHECK(substream))

		return -ENXIO;

	runtime = substream->runtime;

	mutex_lock(&runtime->buffer_mutex);

	snd_pcm_stream_lock_irq(substream);

	switch (runtime->status->state) {

	case SNDRV_PCM_STATE_SETUP:

	case SNDRV_PCM_STATE_PREPARED:

		if (atomic_read(&substream->mmap_count))

			result = -EBADFD;

		break;

	default:

		snd_pcm_stream_unlock_irq(substream);

		return -EBADFD;

		result = -EBADFD;

		break;

	}

	snd_pcm_stream_unlock_irq(substream);

	if (atomic_read(&substream->mmap_count))

		return -EBADFD;

	if (result)

		goto unlock;

	if (substream->ops->hw_free)

		result = substream->ops->hw_free(substream);

	snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN);

	pm_qos_remove_request(&substream->latency_pm_qos_req);

 unlock:

	mutex_unlock(&runtime->buffer_mutex);

	return result;

}