2007-10-08 06:50:34

by Dave Young

Subject: [PATCH] param_sysfs_builtin memchr argument fix

If memchr argument is longer than strlen(kp->name), there will be some
weird result.

Signed-off-by: Dave Young <[email protected]>

---
params.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff -upr linux/kernel/params.c linux.new/kernel/params.c
--- linux/kernel/params.c 2007-10-08 14:30:06.000000000 +0800
+++ linux.new/kernel/params.c 2007-10-08 14:31:22.000000000 +0800
@@ -592,15 +592,21 @@ static void __init param_sysfs_builtin(v

for (i=0; i < __stop___param - __start___param; i++) {
char *dot;
+ int kplen;

kp = &__start___param[i];
+ kplen = strlen(kp->name);

/* We do not handle args without periods. */
- dot = memchr(kp->name, '.', MAX_KBUILD_MODNAME);
+ if (kplen > MAX_KBUILD_MODNAME) {
+ DEBUGP("kernel parameter %s is too long\n", kp->name);
+ continue;
+ }
+ dot = memchr(kp->name, '.', kplen);
if (!dot) {
DEBUGP("couldn't find period in %s\n", kp->name);
continue;
- }
+ }
name_len = dot - kp->name;

/* new kbuild_modname? */

2007-10-08 07:07:38

by Cong Wang

Subject: Re: [PATCH] param_sysfs_builtin memchr argument fix

On Mon, Oct 08, 2007 at 02:50:10PM +0800, Dave Young wrote:
>If memchr argument is longer than strlen(kp->name), there will be some
>weird result.
>
>Signed-off-by: Dave Young <[email protected]>
>
>---
>params.c | 10 ++++++++--
>1 file changed, 8 insertions(+), 2 deletions(-)


Hmm, you used diffstat without -p1?


>
>diff -upr linux/kernel/params.c linux.new/kernel/params.c
>--- linux/kernel/params.c 2007-10-08 14:30:06.000000000 +0800
>+++ linux.new/kernel/params.c 2007-10-08 14:31:22.000000000 +0800
>@@ -592,15 +592,21 @@ static void __init param_sysfs_builtin(v
>
> for (i=0; i < __stop___param - __start___param; i++) {
> char *dot;
>+ int kplen;
>
> kp = &__start___param[i];
>+ kplen = strlen(kp->name);

strlen() returns a size_t value, which is unsigned. ;)

>
> /* We do not handle args without periods. */
>- dot = memchr(kp->name, '.', MAX_KBUILD_MODNAME);
>+ if (kplen > MAX_KBUILD_MODNAME) {
>+ DEBUGP("kernel parameter %s is too long\n", kp->name);
>+ continue;
>+ }
>+ dot = memchr(kp->name, '.', kplen);
> if (!dot) {
> DEBUGP("couldn't find period in %s\n", kp->name);
> continue;
>- }
>+ }

You add an extra whitespace in the end of the line.


--
I try to say goodbye and I choke.
I try to walk away and I stumble.
I play it off, but I’m dreaming of you.
Though I try to hide it, it’s clear that
my world crumbles when you are not here.

2007-10-08 07:17:56

by Dave Young

Subject: Re: [PATCH] param_sysfs_builtin memchr argument fix

Hi,
Thanks for comment.
fixed.

Regards
dave
-----

If memchr argument is longer than strlen(kp->name), there will be some
weird result.

Signed-off-by: Dave Young <[email protected]>

---
kernel/params.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff -upr linux/kernel/params.c linux.new/kernel/params.c
--- linux/kernel/params.c 2007-10-08 14:30:06.000000000 +0800
+++ linux.new/kernel/params.c 2007-10-08 15:13:04.000000000 +0800
@@ -592,11 +592,17 @@ static void __init param_sysfs_builtin(v

for (i=0; i < __stop___param - __start___param; i++) {
char *dot;
+ size_t kplen;

kp = &__start___param[i];
+ kplen = strlen(kp->name);

/* We do not handle args without periods. */
- dot = memchr(kp->name, '.', MAX_KBUILD_MODNAME);
+ if (kplen > MAX_KBUILD_MODNAME) {
+ DEBUGP("kernel parameter %s is too long\n", kp->name);
+ continue;
+ }
+ dot = memchr(kp->name, '.', kplen);
if (!dot) {
DEBUGP("couldn't find period in %s\n", kp->name);
continue;

2007-10-08 17:29:17

by Randy Dunlap

Subject: Re: [PATCH] param_sysfs_builtin memchr argument fix

On Mon, 8 Oct 2007 15:17:30 +0800 Dave Young wrote:

> Hi,
> Thanks for comment.
> fixed.
>
> Regards
> dave
> -----
>
> If memchr argument is longer than strlen(kp->name), there will be some
> weird result.

Just to clarify: this was causing duplicate filenames in sysfs ?


> Signed-off-by: Dave Young <[email protected]>
>
> ---
> kernel/params.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff -upr linux/kernel/params.c linux.new/kernel/params.c
> --- linux/kernel/params.c 2007-10-08 14:30:06.000000000 +0800
> +++ linux.new/kernel/params.c 2007-10-08 15:13:04.000000000 +0800
> @@ -592,11 +592,17 @@ static void __init param_sysfs_builtin(v
>
> for (i=0; i < __stop___param - __start___param; i++) {
> char *dot;
> + size_t kplen;
>
> kp = &__start___param[i];
> + kplen = strlen(kp->name);
>
> /* We do not handle args without periods. */
> - dot = memchr(kp->name, '.', MAX_KBUILD_MODNAME);
> + if (kplen > MAX_KBUILD_MODNAME) {
> + DEBUGP("kernel parameter %s is too long\n", kp->name);

how about
kernel parameter name %s is too long
or
kernel parameter name is too long: %s

(primary is addition of "name")

> + continue;
> + }
> + dot = memchr(kp->name, '.', kplen);
> if (!dot) {
> DEBUGP("couldn't find period in %s\n", kp->name);
> continue;
> -


---
~Randy

2007-10-09 01:21:38

by Dave Young

Subject: Re: [PATCH] param_sysfs_builtin memchr argument fix

> > If memchr argument is longer than strlen(kp->name), there will be some
> > weird result.
>
> Just to clarify: this was causing duplicate filenames in sysfs ?
Yes, it will casuse duplicate filenames in sysfs. For me, the "nousb"
will cause the "usbcore" created twice.
>
>
> > Signed-off-by: Dave Young <[email protected]>
> >
> > ---
> > kernel/params.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff -upr linux/kernel/params.c linux.new/kernel/params.c
> > --- linux/kernel/params.c 2007-10-08 14:30:06.000000000 +0800
> > +++ linux.new/kernel/params.c 2007-10-08 15:13:04.000000000 +0800
> > @@ -592,11 +592,17 @@ static void __init param_sysfs_builtin(v
> >
> > for (i=0; i < __stop___param - __start___param; i++) {
> > char *dot;
> > + size_t kplen;
> >
> > kp = &__start___param[i];
> > + kplen = strlen(kp->name);
> >
> > /* We do not handle args without periods. */
> > - dot = memchr(kp->name, '.', MAX_KBUILD_MODNAME);
> > + if (kplen > MAX_KBUILD_MODNAME) {
> > + DEBUGP("kernel parameter %s is too long\n", kp->name);
>
> how about
> kernel parameter name %s is too long
> or
> kernel parameter name is too long: %s
>
> (primary is addition of "name")
Yes, "name" should be added, thanks.
>
> > + continue;
> > + }
> > + dot = memchr(kp->name, '.', kplen);
> > if (!dot) {
> > DEBUGP("couldn't find period in %s\n", kp->name);
> > continue;
> > -
>

Regards
dave

====================================================

Signed-off-by: Dave Young <[email protected]>

---
kernel/params.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff -upr linux/kernel/params.c linux.new/kernel/params.c
--- linux/kernel/params.c 2007-10-08 14:30:06.000000000 +0800
+++ linux.new/kernel/params.c 2007-10-09 09:16:55.000000000 +0800
@@ -592,11 +592,17 @@ static void __init param_sysfs_builtin(v

for (i=0; i < __stop___param - __start___param; i++) {
char *dot;
+ size_t kplen;

kp = &__start___param[i];
+ kplen = strlen(kp->name);

/* We do not handle args without periods. */
- dot = memchr(kp->name, '.', MAX_KBUILD_MODNAME);
+ if (kplen > MAX_KBUILD_MODNAME) {
+ DEBUGP("kernel parameter name is too long: %s\n", kp->name);
+ continue;
+ }
+ dot = memchr(kp->name, '.', kplen);
if (!dot) {
DEBUGP("couldn't find period in %s\n", kp->name);
continue;