Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ASoC: two trivial fixes
@ 2014-10-19  7:07 Daniel Mack
  2014-10-19  7:07 ` [PATCH 1/2] ASoC: soc-compress: consolidate two identical branches Daniel Mack
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Daniel Mack @ 2014-10-19  7:07 UTC (permalink / raw)
  To: alsa-devel; +Cc: broonie, Daniel Mack

Just two random fixes spotted by Coverity.

Daniel Mack (2):
  ASoC: soc-compress: consolidate two identical branches
  ASoC: fsl: use strncpy() to prevent copying of over-long names

 sound/soc/fsl/fsl_asrc.c |  2 +-
 sound/soc/fsl/fsl_esai.c |  2 +-
 sound/soc/soc-compress.c | 11 ++---------
 3 files changed, 4 insertions(+), 11 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] ASoC: soc-compress: consolidate two identical branches
  2014-10-19  7:07 [PATCH 0/2] ASoC: two trivial fixes Daniel Mack
@ 2014-10-19  7:07 ` Daniel Mack
  2014-10-19  7:07 ` [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names Daniel Mack
  2014-10-22 10:42 ` [PATCH 0/2] ASoC: two trivial fixes Mark Brown
  2 siblings, 0 replies; 6+ messages in thread
From: Daniel Mack @ 2014-10-19  7:07 UTC (permalink / raw)
  To: alsa-devel; +Cc: broonie, Daniel Mack

The actions taken in both branches are identical, so we can simplify the
code. Spotted by Coverity.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 sound/soc/soc-compress.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c
index cecfab3..590a82f 100644
--- a/sound/soc/soc-compress.c
+++ b/sound/soc/soc-compress.c
@@ -258,10 +258,7 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream)
 	list_for_each_entry(dpcm, &fe->dpcm[stream].be_clients, list_be)
 		dpcm->state = SND_SOC_DPCM_LINK_STATE_FREE;
 
-	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
-		dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP);
-	else
-		dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP);
+	dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_STOP);
 
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE;
 	fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO;
@@ -456,11 +453,7 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream,
 	if (ret < 0)
 		goto out;
 
-	if (stream == SNDRV_PCM_STREAM_PLAYBACK)
-		dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);
-	else
-		dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);
-
+	dpcm_dapm_stream_event(fe, stream, SND_SOC_DAPM_STREAM_START);
 	fe->dpcm[stream].state = SND_SOC_DPCM_STATE_PREPARE;
 
 out:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names
  2014-10-19  7:07 [PATCH 0/2] ASoC: two trivial fixes Daniel Mack
  2014-10-19  7:07 ` [PATCH 1/2] ASoC: soc-compress: consolidate two identical branches Daniel Mack
@ 2014-10-19  7:07 ` Daniel Mack
  2014-10-20  5:55   ` Mike Looijmans
  2014-10-22 10:42 ` [PATCH 0/2] ASoC: two trivial fixes Mark Brown
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Mack @ 2014-10-19  7:07 UTC (permalink / raw)
  To: alsa-devel; +Cc: broonie, Daniel Mack

Use strncpy() instead of strcpy(). That's not a security issue, as the
source buffer is taken from DT nodes, but we should still enforce bound
checks. Spotted by Coverity.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 sound/soc/fsl/fsl_asrc.c | 2 +-
 sound/soc/fsl/fsl_esai.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 8221104..dd00b9d 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -792,7 +792,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	asrc_priv->pdev = pdev;
-	strcpy(asrc_priv->name, np->name);
+	strncpy(asrc_priv->name, np->name, sizeof(asrc_priv->name) - 1);
 
 	/* Get the addresses and IRQ */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index a3b29ed..fda1d46 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -729,7 +729,7 @@ static int fsl_esai_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	esai_priv->pdev = pdev;
-	strcpy(esai_priv->name, np->name);
+	strncpy(esai_priv->name, np->name, sizeof(esai_priv->name) - 1);
 
 	if (of_property_read_bool(np, "big-endian"))
 		fsl_esai_regmap_config.val_format_endian = REGMAP_ENDIAN_BIG;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names
  2014-10-19  7:07 ` [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names Daniel Mack
@ 2014-10-20  5:55   ` Mike Looijmans
  2014-10-22 11:50     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Looijmans @ 2014-10-20  5:55 UTC (permalink / raw)
  To: alsa-devel

This hardly improves matters. When the source string is larger than the 
buffer, the destination may not be nul-terminated.
Also, strncpy ALWAYS writes the full buffer so it may be wasting cycles when 
the destination buffer is large.

I'm sure the kernel offers a better alternative. Even "snprintf" is a better 
alternative.

Mike.


On 10/19/2014 09:07 AM, Daniel Mack wrote:
> Use strncpy() instead of strcpy(). That's not a security issue, as the
> source buffer is taken from DT nodes, but we should still enforce bound
> checks. Spotted by Coverity.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
>   sound/soc/fsl/fsl_asrc.c | 2 +-
>   sound/soc/fsl/fsl_esai.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 8221104..dd00b9d 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -792,7 +792,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>
>   	asrc_priv->pdev = pdev;
> -	strcpy(asrc_priv->name, np->name);
> +	strncpy(asrc_priv->name, np->name, sizeof(asrc_priv->name) - 1);
>
>   	/* Get the addresses and IRQ */
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> index a3b29ed..fda1d46 100644
> --- a/sound/soc/fsl/fsl_esai.c
> +++ b/sound/soc/fsl/fsl_esai.c
> @@ -729,7 +729,7 @@ static int fsl_esai_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>
>   	esai_priv->pdev = pdev;
> -	strcpy(esai_priv->name, np->name);
> +	strncpy(esai_priv->name, np->name, sizeof(esai_priv->name) - 1);
>
>   	if (of_property_read_bool(np, "big-endian"))
>   		fsl_esai_regmap_config.val_format_endian = REGMAP_ENDIAN_BIG;
>



Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

Topic zoekt gedreven (embedded) software specialisten!
http://topic.nl/vacatures/topic-zoekt-software-engineers/

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] ASoC: two trivial fixes
  2014-10-19  7:07 [PATCH 0/2] ASoC: two trivial fixes Daniel Mack
  2014-10-19  7:07 ` [PATCH 1/2] ASoC: soc-compress: consolidate two identical branches Daniel Mack
  2014-10-19  7:07 ` [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names Daniel Mack
@ 2014-10-22 10:42 ` Mark Brown
  2 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-10-22 10:42 UTC (permalink / raw)
  To: Daniel Mack; +Cc: alsa-devel


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

On Sun, Oct 19, 2014 at 09:07:34AM +0200, Daniel Mack wrote:
> Just two random fixes spotted by Coverity.

Applied both.  I'd been holding off on these waiting for the developers
of the relevant code to review but I now notice that you didn't CC them
which is probably why they didn't respond!

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

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names
  2014-10-20  5:55   ` Mike Looijmans
@ 2014-10-22 11:50     ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2014-10-22 11:50 UTC (permalink / raw)
  To: Mike Looijmans; +Cc: alsa-devel

At Mon, 20 Oct 2014 07:55:22 +0200,
Mike Looijmans wrote:
> 
> This hardly improves matters. When the source string is larger than the 
> buffer, the destination may not be nul-terminated.
> Also, strncpy ALWAYS writes the full buffer so it may be wasting cycles when 
> the destination buffer is large.

Indeed, strncpy() should be avoided.

> I'm sure the kernel offers a better alternative. Even "snprintf" is a better 
> alternative.

strlcpy() is the best choice.


Takashi


> 
> Mike.
> 
> 
> On 10/19/2014 09:07 AM, Daniel Mack wrote:
> > Use strncpy() instead of strcpy(). That's not a security issue, as the
> > source buffer is taken from DT nodes, but we should still enforce bound
> > checks. Spotted by Coverity.
> >
> > Signed-off-by: Daniel Mack <daniel@zonque.org>
> > ---
> >   sound/soc/fsl/fsl_asrc.c | 2 +-
> >   sound/soc/fsl/fsl_esai.c | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > index 8221104..dd00b9d 100644
> > --- a/sound/soc/fsl/fsl_asrc.c
> > +++ b/sound/soc/fsl/fsl_asrc.c
> > @@ -792,7 +792,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >
> >   	asrc_priv->pdev = pdev;
> > -	strcpy(asrc_priv->name, np->name);
> > +	strncpy(asrc_priv->name, np->name, sizeof(asrc_priv->name) - 1);
> >
> >   	/* Get the addresses and IRQ */
> >   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
> > index a3b29ed..fda1d46 100644
> > --- a/sound/soc/fsl/fsl_esai.c
> > +++ b/sound/soc/fsl/fsl_esai.c
> > @@ -729,7 +729,7 @@ static int fsl_esai_probe(struct platform_device *pdev)
> >   		return -ENOMEM;
> >
> >   	esai_priv->pdev = pdev;
> > -	strcpy(esai_priv->name, np->name);
> > +	strncpy(esai_priv->name, np->name, sizeof(esai_priv->name) - 1);
> >
> >   	if (of_property_read_bool(np, "big-endian"))
> >   		fsl_esai_regmap_config.val_format_endian = REGMAP_ENDIAN_BIG;
> >
> 
> 
> 
> Met vriendelijke groet / kind regards,
> 
> Mike Looijmans
> 
> TOPIC Embedded Systems
> Eindhovenseweg 32-C, NL-5683 KH Best
> Postbus 440, NL-5680 AK Best
> Telefoon: (+31) (0) 499 33 69 79
> Telefax:  (+31) (0) 499 33 69 70
> E-mail: mike.looijmans@topic.nl
> Website: www.topic.nl
> 
> Please consider the environment before printing this e-mail
> 
> Topic zoekt gedreven (embedded) software specialisten!
> http://topic.nl/vacatures/topic-zoekt-software-engineers/
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-10-22 11:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-19  7:07 [PATCH 0/2] ASoC: two trivial fixes Daniel Mack
2014-10-19  7:07 ` [PATCH 1/2] ASoC: soc-compress: consolidate two identical branches Daniel Mack
2014-10-19  7:07 ` [PATCH 2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names Daniel Mack
2014-10-20  5:55   ` Mike Looijmans
2014-10-22 11:50     ` Takashi Iwai
2014-10-22 10:42 ` [PATCH 0/2] ASoC: two trivial fixes Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox