[PATCH 1/6] ALSA: PCM: Remove redundant null check before kfree

[PATCH 1/6] ALSA: PCM: Remove redundant null check before kfree

[Sachin Kamat]

kfree on a null pointer is a no-op.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested and based on linux-next 20121115.
---
 sound/core/pcm.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 030102c..61798f8 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -981,8 +981,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream)
 		       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control)));
 	kfree(runtime->hw_constraints.rules);
 #ifdef CONFIG_SND_PCM_XRUN_DEBUG
-	if (runtime->hwptr_log)
-		kfree(runtime->hwptr_log);
+	kfree(runtime->hwptr_log);
 #endif
 	kfree(runtime);
 	substream->runtime = NULL;
-- 
1.7.4.1

[PATCH 2/6] ALSA: pcm: Fix return code in pcm_native.c

[Sachin Kamat]

Return the value obtained from snd_pcm_hw_constraint_minmax() instead
of -EINVAL. Silences the following smatch warning:
sound/core/pcm_native.c:2003 snd_pcm_hw_constraints_complete() info:
why not propagate 'err' from snd_pcm_hw_constraint_minmax() instead of -22?

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested and based on linux-next 20121115.
---
 sound/core/pcm_native.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index c76f6375..09b4286 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2000,7 +2000,7 @@ int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream)
 	if (runtime->dma_bytes) {
 		err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 0, runtime->dma_bytes);
 		if (err < 0)
-			return -EINVAL;
+			return err;
 	}
 
 	if (!(hw->rates & (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))) {
-- 
1.7.4.1

Re: [PATCH 2/6] ALSA: pcm: Fix return code in pcm_native.c

[Takashi Iwai]

At Wed, 21 Nov 2012 14:36:55 +0530,
Sachin Kamat wrote:
> 
> Return the value obtained from snd_pcm_hw_constraint_minmax() instead
> of -EINVAL. Silences the following smatch warning:
> sound/core/pcm_native.c:2003 snd_pcm_hw_constraints_complete() info:
> why not propagate 'err' from snd_pcm_hw_constraint_minmax() instead of -22?
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Thanks, applied.


Takashi

> ---
> Build tested and based on linux-next 20121115.
> ---
>  sound/core/pcm_native.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index c76f6375..09b4286 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2000,7 +2000,7 @@ int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream)
>  	if (runtime->dma_bytes) {
>  		err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, 0, runtime->dma_bytes);
>  		if (err < 0)
> -			return -EINVAL;
> +			return err;
>  	}
>  
>  	if (!(hw->rates & (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS))) {
> -- 
> 1.7.4.1
> 

[PATCH 3/6] ASoC: Samsung-dma: Fix potential NULL pointer dereference

[Sachin Kamat]

'substream' was dereferenced before the NULL check.
Moved the check earlier to prevent NULL pointer dereference.

Cc: Sangbeom Kim <sbkim73@samsung.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested and based on linux-next 20121115.
---
 sound/soc/samsung/dma.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c
index b70964e..e59a4f4 100644
--- a/sound/soc/samsung/dma.c
+++ b/sound/soc/samsung/dma.c
@@ -114,17 +114,23 @@ static void dma_enqueue(struct snd_pcm_substream *substream)
 static void audio_buffdone(void *data)
 {
 	struct snd_pcm_substream *substream = data;
-	struct runtime_data *prtd = substream->runtime->private_data;
+	struct runtime_data *prtd;
 
 	pr_debug("Entered %s\n", __func__);
 
+	if (substream) {
+		prtd = substream->runtime->private_data;
+	} else {
+		pr_err("%s: Null data received\n", __func__);
+		return;
+	}
+
 	if (prtd->state & ST_RUNNING) {
 		prtd->dma_pos += prtd->dma_period;
 		if (prtd->dma_pos >= prtd->dma_end)
 			prtd->dma_pos = prtd->dma_start;
 
-		if (substream)
-			snd_pcm_period_elapsed(substream);
+		snd_pcm_period_elapsed(substream);
 
 		spin_lock(&prtd->lock);
 		if (!samsung_dma_has_circular()) {
-- 
1.7.4.1

Re: [PATCH 3/6] ASoC: Samsung-dma: Fix potential NULL pointer dereference

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 450 bytes --]

On Wed, Nov 21, 2012 at 02:36:56PM +0530, Sachin Kamat wrote:

>  static void audio_buffdone(void *data)
>  {
>  	struct snd_pcm_substream *substream = data;
> -	struct runtime_data *prtd = substream->runtime->private_data;
> +	struct runtime_data *prtd;

This should be a BUG_ON() or something rather than just a simple warning
- the checks here are redundant, there's no way we should ever end up
completing a buffer without an associated runtime.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 4/6] ALSA: usb: Fix potential NULL pointer dereference in comm.c

[Sachin Kamat]

'rt' was dereferenced before the NULL check.
Moved the code after the check.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested and based on linux-next 20121115.
---
 sound/usb/6fire/comm.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c
index 6c3d531..1a75c36 100644
--- a/sound/usb/6fire/comm.c
+++ b/sound/usb/6fire/comm.c
@@ -129,12 +129,13 @@ int __devinit usb6fire_comm_init(struct sfire_chip *chip)
 {
 	struct comm_runtime *rt = kzalloc(sizeof(struct comm_runtime),
 			GFP_KERNEL);
-	struct urb *urb = &rt->receiver;
+	struct urb *urb;
 	int ret;
 
 	if (!rt)
 		return -ENOMEM;
 
+	urb = &rt->receiver;
 	rt->serial = 1;
 	rt->chip = chip;
 	usb_init_urb(urb);
-- 
1.7.4.1

Re: [PATCH 4/6] ALSA: usb: Fix potential NULL pointer dereference in comm.c

[Takashi Iwai]

At Wed, 21 Nov 2012 14:36:57 +0530,
Sachin Kamat wrote:
> 
> 'rt' was dereferenced before the NULL check.
> Moved the code after the check.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Applied (with a minor change in the subject line).
Thanks.


Takashi


> ---
> Build tested and based on linux-next 20121115.
> ---
>  sound/usb/6fire/comm.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c
> index 6c3d531..1a75c36 100644
> --- a/sound/usb/6fire/comm.c
> +++ b/sound/usb/6fire/comm.c
> @@ -129,12 +129,13 @@ int __devinit usb6fire_comm_init(struct sfire_chip *chip)
>  {
>  	struct comm_runtime *rt = kzalloc(sizeof(struct comm_runtime),
>  			GFP_KERNEL);
> -	struct urb *urb = &rt->receiver;
> +	struct urb *urb;
>  	int ret;
>  
>  	if (!rt)
>  		return -ENOMEM;
>  
> +	urb = &rt->receiver;
>  	rt->serial = 1;
>  	rt->chip = chip;
>  	usb_init_urb(urb);
> -- 
> 1.7.4.1
> 

[PATCH 5/6] ALSA: usb-audio: Return -ENOMEM on memory alloc failure in format.c

[Sachin Kamat]

Return -ENOMEM instead of -1 to silence the following smatch warning:
sound/usb/format.c:170 parse_audio_format_rates_v1() warn:
returning -1 instead of -ENOMEM is sloppy

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested and based on linux-next 20121115.
---
 sound/usb/format.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/usb/format.c b/sound/usb/format.c
index ddfef57..d033d4d 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -167,7 +167,7 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
 		fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
 		if (fp->rate_table == NULL) {
 			snd_printk(KERN_ERR "cannot malloc\n");
-			return -1;
+			return -ENOMEM;
 		}
 
 		fp->nr_rates = 0;
-- 
1.7.4.1

Re: [PATCH 5/6] ALSA: usb-audio: Return -ENOMEM on memory alloc failure in format.c

[Takashi Iwai]

At Wed, 21 Nov 2012 14:36:58 +0530,
Sachin Kamat wrote:
> 
> Return -ENOMEM instead of -1 to silence the following smatch warning:
> sound/usb/format.c:170 parse_audio_format_rates_v1() warn:
> returning -1 instead of -ENOMEM is sloppy
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

There are some other places returning -1 around this.
Could you fix them as well to sensitive values?


thanks,

Takashi


> ---
> Build tested and based on linux-next 20121115.
> ---
>  sound/usb/format.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/usb/format.c b/sound/usb/format.c
> index ddfef57..d033d4d 100644
> --- a/sound/usb/format.c
> +++ b/sound/usb/format.c
> @@ -167,7 +167,7 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
>  		fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
>  		if (fp->rate_table == NULL) {
>  			snd_printk(KERN_ERR "cannot malloc\n");
> -			return -1;
> +			return -ENOMEM;
>  		}
>  
>  		fp->nr_rates = 0;
> -- 
> 1.7.4.1
> 

Re: [PATCH 5/6] ALSA: usb-audio: Return -ENOMEM on memory alloc failure in format.c

[Sachin Kamat]

On 21 November 2012 15:15, Takashi Iwai <tiwai@suse.de> wrote:
> At Wed, 21 Nov 2012 14:36:58 +0530,
> Sachin Kamat wrote:
>>
>> Return -ENOMEM instead of -1 to silence the following smatch warning:
>> sound/usb/format.c:170 parse_audio_format_rates_v1() warn:
>> returning -1 instead of -ENOMEM is sloppy
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>
> There are some other places returning -1 around this.
> Could you fix them as well to sensitive values?

Ok. Will update and re-send this patch.
>
>
> thanks,
>
> Takashi
>
>
>> ---
>> Build tested and based on linux-next 20121115.
>> ---
>>  sound/usb/format.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/sound/usb/format.c b/sound/usb/format.c
>> index ddfef57..d033d4d 100644
>> --- a/sound/usb/format.c
>> +++ b/sound/usb/format.c
>> @@ -167,7 +167,7 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof
>>               fp->rate_table = kmalloc(sizeof(int) * nr_rates, GFP_KERNEL);
>>               if (fp->rate_table == NULL) {
>>                       snd_printk(KERN_ERR "cannot malloc\n");
>> -                     return -1;
>> +                     return -ENOMEM;
>>               }
>>
>>               fp->nr_rates = 0;
>> --
>> 1.7.4.1
>>



-- 
With warm regards,
Sachin

[PATCH 6/6] ASoC: dapm: Fix potential NULL pointer dereference

[Sachin Kamat]

'w' should not be dereferenced when it is NULL.

Cc: Liam Girdwood <lrg@ti.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
Build tested and based on linux-next 20121115.
---
 sound/soc/soc-dapm.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 6e35bca..5bde5d4 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3326,9 +3326,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
 		if (!w) {
 			dev_err(dapm->dev, "Failed to create %s widget\n",
 				dai->driver->playback.stream_name);
+		} else {
+			w->priv = dai;
 		}
-
-		w->priv = dai;
 		dai->playback_widget = w;
 	}
 
@@ -3344,9 +3344,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
 		if (!w) {
 			dev_err(dapm->dev, "Failed to create %s widget\n",
 				dai->driver->capture.stream_name);
+		} else {
+			w->priv = dai;
 		}
-
-		w->priv = dai;
 		dai->capture_widget = w;
 	}
 
-- 
1.7.4.1

Re: [PATCH 6/6] ASoC: dapm: Fix potential NULL pointer dereference

[Sachin Kamat]

Not sure if this patch got lost.
Just a gentle reminder.

On 21 November 2012 14:36, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> 'w' should not be dereferenced when it is NULL.
>
> Cc: Liam Girdwood <lrg@ti.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> Build tested and based on linux-next 20121115.
> ---
>  sound/soc/soc-dapm.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index 6e35bca..5bde5d4 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -3326,9 +3326,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
>                 if (!w) {
>                         dev_err(dapm->dev, "Failed to create %s widget\n",
>                                 dai->driver->playback.stream_name);
> +               } else {
> +                       w->priv = dai;
>                 }
> -
> -               w->priv = dai;
>                 dai->playback_widget = w;
>         }
>
> @@ -3344,9 +3344,9 @@ int snd_soc_dapm_new_dai_widgets(struct snd_soc_dapm_context *dapm,
>                 if (!w) {
>                         dev_err(dapm->dev, "Failed to create %s widget\n",
>                                 dai->driver->capture.stream_name);
> +               } else {
> +                       w->priv = dai;
>                 }
> -
> -               w->priv = dai;
>                 dai->capture_widget = w;
>         }
>
> --
> 1.7.4.1
>



-- 
With warm regards,
Sachin

Re: [PATCH 6/6] ASoC: dapm: Fix potential NULL pointer dereference

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 254 bytes --]

On Wed, Nov 28, 2012 at 10:13:58AM +0530, Sachin Kamat wrote:
> Not sure if this patch got lost.
> Just a gentle reminder.

Don't send contentless pings or top post.  If the patch has been lost
it's not helpful to have a copy covered in quotation marks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 6/6] ASoC: dapm: Fix potential NULL pointer dereference

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 357 bytes --]

On Wed, Nov 21, 2012 at 02:36:59PM +0530, Sachin Kamat wrote:

>  		if (!w) {
>  			dev_err(dapm->dev, "Failed to create %s widget\n",
>  				dai->driver->playback.stream_name);
> +		} else {
> +			w->priv = dai;
>  		}

This isn't a particularly sensible fix, we should be erroring out or
something rather than limping along with the rest of the function.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/6] ALSA: PCM: Remove redundant null check before kfree

[Takashi Iwai]

At Wed, 21 Nov 2012 14:36:54 +0530,
Sachin Kamat wrote:
> 
> kfree on a null pointer is a no-op.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Thanks, applied.


Takashi

> ---
> Build tested and based on linux-next 20121115.
> ---
>  sound/core/pcm.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/core/pcm.c b/sound/core/pcm.c
> index 030102c..61798f8 100644
> --- a/sound/core/pcm.c
> +++ b/sound/core/pcm.c
> @@ -981,8 +981,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream)
>  		       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control)));
>  	kfree(runtime->hw_constraints.rules);
>  #ifdef CONFIG_SND_PCM_XRUN_DEBUG
> -	if (runtime->hwptr_log)
> -		kfree(runtime->hwptr_log);
> +	kfree(runtime->hwptr_log);
>  #endif
>  	kfree(runtime);
>  	substream->runtime = NULL;
> -- 
> 1.7.4.1
>