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

Commit fa2b30af authored by Julia Lawall's avatar Julia Lawall Committed by Takashi Iwai
Browse files

ALSA: sound/pci/ctxfi/ctpcm.c: Remove potential for use after free 

In each function, the value apcm is stored in the private_data field of
runtime.  At the same time the function ct_atc_pcm_free_substream is stored
in the private_free field of the same structure.  ct_atc_pcm_free_substream
dereferences and ultimately frees the value in the private_data field.  But
each function can exit in an error case with apcm having been freed, in
which case a subsequent call to the private_free function would perform a
dereference after free.  On the other hand, if the private_free field is
not initialized, it is NULL, and not invoked (see snd_pcm_detach_substream
in sound/core/pcm.c).  To avoid the introduction of a dangling pointer, the
initializations of the private_data and private_free fields are moved to
the end of the function, past any possible free of apcm.  This is safe
because the previous calls to snd_pcm_hw_constraint_integer and
snd_pcm_hw_constraint_minmax, which take runtime as an argument, do not
refer to either of these fields.

In each function, there is one error case where apcm needs to be freed, and
a call to kfree is added.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/

)

// <smpl>
@@
expression e,e1,e2,e3;
identifier f,free1,free2;
expression a;
@@

*e->f = a
... when != e->f = e1
    when any
if (...) {
  ... when != free1(...,e,...)
      when != e->f = e2
* kfree(a)
  ... when != free2(...,e,...)
      when != e->f = e3
}
// </smpl>

Signed-off-by: default avatarJulia Lawall <julia@diku.dk>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent e9161512

sound/pci/ctxfi/ctpcm.c

+10 −6
Original line number Original line Diff line number Diff line
@@ -129,8 +129,6 @@ static int ct_pcm_playback_open(struct snd_pcm_substream *substream) 




	apcm->substream = substream;

	apcm->substream = substream;

	apcm->interrupt = ct_atc_pcm_interrupt;

	apcm->interrupt = ct_atc_pcm_interrupt;

	runtime->private_data = apcm;

	runtime->private_free = ct_atc_pcm_free_substream;

	if (IEC958 == substream->pcm->device) {

	if (IEC958 == substream->pcm->device) {

		runtime->hw = ct_spdif_passthru_playback_hw;

		runtime->hw = ct_spdif_passthru_playback_hw;

		atc->spdif_out_passthru(atc, 1);

		atc->spdif_out_passthru(atc, 1);
@@ -155,8 +153,12 @@ static int ct_pcm_playback_open(struct snd_pcm_substream *substream) 
	}

	}





	apcm->timer = ct_timer_instance_new(atc->timer, apcm);

	apcm->timer = ct_timer_instance_new(atc->timer, apcm);

	if (!apcm->timer)

	if (!apcm->timer) {

		kfree(apcm);

		return -ENOMEM;

		return -ENOMEM;

	}

	runtime->private_data = apcm;

	runtime->private_free = ct_atc_pcm_free_substream;





	return 0;

	return 0;

}

}
@@ -278,8 +280,6 @@ static int ct_pcm_capture_open(struct snd_pcm_substream *substream) 
	apcm->started = 0;

	apcm->started = 0;

	apcm->substream = substream;

	apcm->substream = substream;

	apcm->interrupt = ct_atc_pcm_interrupt;

	apcm->interrupt = ct_atc_pcm_interrupt;

	runtime->private_data = apcm;

	runtime->private_free = ct_atc_pcm_free_substream;

	runtime->hw = ct_pcm_capture_hw;

	runtime->hw = ct_pcm_capture_hw;

	runtime->hw.rate_max = atc->rsr * atc->msr;

	runtime->hw.rate_max = atc->rsr * atc->msr;
@@ -298,8 +298,12 @@ static int ct_pcm_capture_open(struct snd_pcm_substream *substream) 
	}

	}





	apcm->timer = ct_timer_instance_new(atc->timer, apcm);

	apcm->timer = ct_timer_instance_new(atc->timer, apcm);

	if (!apcm->timer)

	if (!apcm->timer) {

		kfree(apcm);

		return -ENOMEM;

		return -ENOMEM;

	}

	runtime->private_data = apcm;

	runtime->private_free = ct_atc_pcm_free_substream;





	return 0;

	return 0;

}

}