diff mbox

[2/2] ASoC: fsl: use strncpy() to prevent copying of over-long names

Message ID 1413702456-8624-3-git-send-email-daniel@zonque.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Mack Oct. 19, 2014, 7:07 a.m. UTC
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(-)

Comments

Mike Looijmans Oct. 20, 2014, 5:55 a.m. UTC | #1
?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/
Takashi Iwai Oct. 22, 2014, 11:50 a.m. UTC | #2
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
diff mbox

Patch

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;