* [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 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
* 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
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