Patchwork [U-Boot,v3] miiphy: use strncpy() not sprintf()

login
register
mail settings
Submitter Laurence Withers
Date July 15, 2011, 11:01 a.m.
Message ID <1310727678-30367-1-git-send-email-lwithers@guralp.com>
Download mbox | patch
Permalink /patch/104786/
State Accepted
Headers show

Comments

Laurence Withers - July 15, 2011, 11:01 a.m.
In miiphy_register() the new device's name was initialised by passing a
string parameter as the format string to sprintf(). As this would cause
problems if it ever contained a '%' symbol, switch to using strncpy()
instead.

Signed-off-by: Laurence Withers <lwithers@guralp.com>
Cc: Andy Fleming <afleming@freescale.com>
---
Changes for v2:
 - Use strncpy() rather than plain strcpy() for extra safety.

Changes for v3:
 - Use BUG_ON() as an additional safety measure to ensure the name never
   exceeds the buffer size MDIO_NAME_LEN, simplifying the previous test.
---
 common/miiphyutil.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)
Mike Frysinger - July 18, 2011, 6:03 p.m.
On Fri, Jul 15, 2011 at 07:01, Laurence Withers wrote:
> In miiphy_register() the new device's name was initialised by passing a
> string parameter as the format string to sprintf(). As this would cause
> problems if it ever contained a '%' symbol, switch to using strncpy()
> instead.

Acked-by: Mike Frysinger <vapier@gentoo.org>
-mike
Wolfgang Denk - July 26, 2011, noon
Dear Laurence Withers,

In message <1310727678-30367-1-git-send-email-lwithers@guralp.com> you wrote:
> In miiphy_register() the new device's name was initialised by passing a
> string parameter as the format string to sprintf(). As this would cause
> problems if it ever contained a '%' symbol, switch to using strncpy()
> instead.
> 
> Signed-off-by: Laurence Withers <lwithers@guralp.com>
> Cc: Andy Fleming <afleming@freescale.com>
> ---
> Changes for v2:
>  - Use strncpy() rather than plain strcpy() for extra safety.
> 
> Changes for v3:
>  - Use BUG_ON() as an additional safety measure to ensure the name never
>    exceeds the buffer size MDIO_NAME_LEN, simplifying the previous test.
> ---
>  common/miiphyutil.c |   14 ++++----------
>  1 files changed, 4 insertions(+), 10 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/common/miiphyutil.c b/common/miiphyutil.c
index bcab74e..35ad357 100644
--- a/common/miiphyutil.c
+++ b/common/miiphyutil.c
@@ -111,7 +111,8 @@  void miiphy_register(const char *name,
 {
 	struct mii_dev *new_dev;
 	struct legacy_mii_dev *ldev;
-	unsigned int name_len;
+
+	BUG_ON(strlen(name) >= MDIO_NAME_LEN);
 
 	/* check if we have unique name */
 	new_dev = miiphy_get_dev_by_name(name);
@@ -121,14 +122,6 @@  void miiphy_register(const char *name,
 	}
 
 	/* allocate memory */
-	name_len = strlen(name);
-	if (name_len > MDIO_NAME_LEN - 1) {
-		/* Hopefully this won't happen, but if it does, we'll know */
-		printf("miiphy_register: MDIO name was longer than %d\n",
-			MDIO_NAME_LEN);
-		return;
-	}
-
 	new_dev = mdio_alloc();
 	ldev = malloc(sizeof(*ldev));
 
@@ -141,7 +134,8 @@  void miiphy_register(const char *name,
 	/* initalize mii_dev struct fields */
 	new_dev->read = legacy_miiphy_read;
 	new_dev->write = legacy_miiphy_write;
-	sprintf(new_dev->name, name);
+	strncpy(new_dev->name, name, MDIO_NAME_LEN);
+	new_dev->name[MDIO_NAME_LEN - 1] = 0;
 	ldev->read = read;
 	ldev->write = write;
 	new_dev->priv = ldev;