diff mbox

[v6,10/15] ASoC: hdac_hdmi: Fix possible memory leak in hw_params

Message ID 1455243375-16067-11-git-send-email-subhransu.s.prusty@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Subhransu S. Prusty Feb. 12, 2016, 2:16 a.m. UTC 
It's possible for hw_params to be called two times. So add NULL
check to prevent memory leak.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Mark Brown Feb. 15, 2016, 8:57 p.m. UTC | #1 
On Fri, Feb 12, 2016 at 07:46:10AM +0530, Subhransu S. Prusty wrote:
> It's possible for hw_params to be called two times. So add NULL
> check to prevent memory leak.

Another fix?

> -	dd = kzalloc(sizeof(*dd), GFP_KERNEL);
> -	if (!dd)
> -		return -ENOMEM;
> +	dd = (struct hdac_ext_dma_params *)
> +		snd_soc_dai_get_dma_data(dai, substream);

If you need to cast away from void * that suggests a problem and it'd
look a lot neater too.
Vinod Koul Feb. 16, 2016, 2:58 a.m. UTC | #2 
On Mon, Feb 15, 2016 at 08:57:26PM +0000, Mark Brown wrote:
> On Fri, Feb 12, 2016 at 07:46:10AM +0530, Subhransu S. Prusty wrote:
> > It's possible for hw_params to be called two times. So add NULL
> > check to prevent memory leak.
> 
> Another fix?

Yes but can go in 4.6 as HDMI codec does not get created in 4.5 code. That
is why I didn't send this one in last fixes series

> 
> > -	dd = kzalloc(sizeof(*dd), GFP_KERNEL);
> > -	if (!dd)
> > -		return -ENOMEM;
> > +	dd = (struct hdac_ext_dma_params *)
> > +		snd_soc_dai_get_dma_data(dai, substream);
> 
> If you need to cast away from void * that suggests a problem and it'd
> look a lot neater too.

I suspect there is no need for cast, we seem to ahve a habit for that, let
us check that
diff mbox

Patch

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 36f1200..40db292 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -405,9 +405,14 @@  static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
 		return -ENODEV;
 	}
 
-	dd = kzalloc(sizeof(*dd), GFP_KERNEL);
-	if (!dd)
-		return -ENOMEM;
+	dd = (struct hdac_ext_dma_params *)
+		snd_soc_dai_get_dma_data(dai, substream);
+	if (!dd) {
+		dd = kzalloc(sizeof(*dd), GFP_KERNEL);
+		if (!dd)
+			return -ENOMEM;
+	}
+
 	dd->format = snd_hdac_calc_stream_format(params_rate(hparams),
 			params_channels(hparams), params_format(hparams),
 			24, 0);
@@ -433,9 +438,11 @@  static int hdac_hdmi_playback_cleanup(struct snd_pcm_substream *substream,
 				AC_VERB_SET_STREAM_FORMAT, 0);
 
 	dd = (struct hdac_ext_dma_params *)snd_soc_dai_get_dma_data(dai, substream);
-	snd_soc_dai_set_dma_data(dai, substream, NULL);
 
-	kfree(dd);
+	if (dd) {
+		snd_soc_dai_set_dma_data(dai, substream, NULL);
+		kfree(dd);
+	}
 
 	return 0;
 }