| Message ID | 20090310211352.16425.44160.stgit@localhost (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | Tony Lindgren |
| Headers | show |
On Tue, Mar 10, 2009 at 02:13:52PM -0700, Tony Lindgren wrote: > From: Adrian Hunter <adrian.hunter@nokia.com> > > Signed-off-by: Adrian Hunter <adrian.hunter@nokia.com> > Acked-by: David Brownell <dbrownell@users.sourceforge.net> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > arch/arm/mach-omap2/mmc-twl4030.c | 5 ++++- > arch/arm/mach-omap2/mmc-twl4030.h | 1 + > 2 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-omap2/mmc-twl4030.c b/arch/arm/mach-omap2/mmc-twl4030.c > index 9f53d22..9831b2b 100644 > --- a/arch/arm/mach-omap2/mmc-twl4030.c > +++ b/arch/arm/mach-omap2/mmc-twl4030.c > @@ -402,7 +402,10 @@ void __init twl4030_mmc_init(struct twl4030_hsmmc_info *controllers) > return; > } > > - sprintf(twl->name, "mmc%islot%i", c->mmc, 1); > + if (c->name) > + strncpy(twl->name, c->name, HSMMC_NAME_LEN); Bug. strncpy can result in a non-null terminated string, and it just so happens that twl->name has been declared as being HSMMC_NAME_LEN+1 in size. That's rather unsafe. Using strlcpy() and sizeof(twl->name) instead will ensure that no matter what happens to the size of the array declaration, the code remains correct. > diff --git a/arch/arm/mach-omap2/mmc-twl4030.h b/arch/arm/mach-omap2/mmc-twl4030.h > index 0aa1686..ea59e86 100644 > --- a/arch/arm/mach-omap2/mmc-twl4030.h > +++ b/arch/arm/mach-omap2/mmc-twl4030.h > @@ -14,6 +14,7 @@ struct twl4030_hsmmc_info { > bool cover_only; /* No card detect - just cover switch */ > int gpio_cd; /* or -EINVAL */ > int gpio_wp; /* or -EINVAL */ > + char *name; /* or NULL for default */ const? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux wrote: > On Tue, Mar 10, 2009 at 02:13:52PM -0700, Tony Lindgren wrote: >> From: Adrian Hunter <adrian.hunter@nokia.com> >> >> Signed-off-by: Adrian Hunter <adrian.hunter@nokia.com> >> Acked-by: David Brownell <dbrownell@users.sourceforge.net> >> Signed-off-by: Tony Lindgren <tony@atomide.com> >> --- >> arch/arm/mach-omap2/mmc-twl4030.c | 5 ++++- >> arch/arm/mach-omap2/mmc-twl4030.h | 1 + >> 2 files changed, 5 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/mmc-twl4030.c b/arch/arm/mach-omap2/mmc-twl4030.c >> index 9f53d22..9831b2b 100644 >> --- a/arch/arm/mach-omap2/mmc-twl4030.c >> +++ b/arch/arm/mach-omap2/mmc-twl4030.c >> @@ -402,7 +402,10 @@ void __init twl4030_mmc_init(struct twl4030_hsmmc_info *controllers) >> return; >> } >> >> - sprintf(twl->name, "mmc%islot%i", c->mmc, 1); >> + if (c->name) >> + strncpy(twl->name, c->name, HSMMC_NAME_LEN); > > Bug. strncpy can result in a non-null terminated string, and it just so > happens that twl->name has been declared as being HSMMC_NAME_LEN+1 in size. > Well to be pedantic, it is not actually a "bug". twl->name is zero-filled so HSMMC_NAME_LEN+1 guarantees that it is null terminated. > That's rather unsafe. Using strlcpy() and sizeof(twl->name) instead will > ensure that no matter what happens to the size of the array declaration, > the code remains correct. Unless the programmer decides to make twl->name a pointer to allocated memory - then sizeof(twl->name) is no good. You could use ARRAY_SIZE(twl->name) which (for gcc) includes a check that twl->name is an array. > >> diff --git a/arch/arm/mach-omap2/mmc-twl4030.h b/arch/arm/mach-omap2/mmc-twl4030.h >> index 0aa1686..ea59e86 100644 >> --- a/arch/arm/mach-omap2/mmc-twl4030.h >> +++ b/arch/arm/mach-omap2/mmc-twl4030.h >> @@ -14,6 +14,7 @@ struct twl4030_hsmmc_info { >> bool cover_only; /* No card detect - just cover switch */ >> int gpio_cd; /* or -EINVAL */ >> int gpio_wp; /* or -EINVAL */ >> + char *name; /* or NULL for default */ > > const? > OK -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/mmc-twl4030.c b/arch/arm/mach-omap2/mmc-twl4030.c index 9f53d22..9831b2b 100644 --- a/arch/arm/mach-omap2/mmc-twl4030.c +++ b/arch/arm/mach-omap2/mmc-twl4030.c @@ -402,7 +402,10 @@ void __init twl4030_mmc_init(struct twl4030_hsmmc_info *controllers) return; } - sprintf(twl->name, "mmc%islot%i", c->mmc, 1); + if (c->name) + strncpy(twl->name, c->name, HSMMC_NAME_LEN); + else + sprintf(twl->name, "mmc%islot%i", c->mmc, 1); mmc->slots[0].name = twl->name; mmc->nr_slots = 1; mmc->slots[0].wires = c->wires; diff --git a/arch/arm/mach-omap2/mmc-twl4030.h b/arch/arm/mach-omap2/mmc-twl4030.h index 0aa1686..ea59e86 100644 --- a/arch/arm/mach-omap2/mmc-twl4030.h +++ b/arch/arm/mach-omap2/mmc-twl4030.h @@ -14,6 +14,7 @@ struct twl4030_hsmmc_info { bool cover_only; /* No card detect - just cover switch */ int gpio_cd; /* or -EINVAL */ int gpio_wp; /* or -EINVAL */ + char *name; /* or NULL for default */ struct device *dev; /* returned: pointer to mmc adapter */ };