All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.