Re: [PATCH] Fix undefined/miscompiled construct in kernel parameters

From: Rusty Russell (rusty@rustcorp.com.au)
Date: Sun Jun 15 2003 - 19:23:41 EST

In message <Pine.LNX.4.44.0306150951060.8088-100000@home.transmeta.com> you wri
te:
>
> On Sun, 15 Jun 2003, Andi Kleen wrote:
> >
> > The parse_args call in init/main.c does pointer arithmetic between two
> > different external symbols. This is undefined in C (only pointer arthmetic
> > in the same symbol is defined) and gets miscompiled on AMD64 with gcc 3.2,
>
> That's silly. You're making the code less readable, with some silly
> parameters. Why does it get miscompiled on amd64, and let's just fix that
> one.

AFAICT, Roman's fix is correct; Richard admonished me in the past for
such code, IIRC, but this one slipped through.

Since Andi reports that even that doesn't work for x86-64, I'd say
apply this patch based on his: it's an arbitrary change anyway.

The compiler should be fixed, too, but if this is the only compiler
bug which effects the kernel, we should consider ourselves lucky.

(Andi: your module.c patch was a little convoluted).
Cheers,
Rusty.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.71-bk1/include/linux/moduleparam.h working-2.5.71-bk1-param/include/linux/moduleparam.h
--- linux-2.5.71-bk1/include/linux/moduleparam.h 2003-02-07 19:20:01.000000000 +1100
+++ working-2.5.71-bk1-param/include/linux/moduleparam.h 2003-06-16 10:18:25.000000000 +1000
@@ -67,7 +67,7 @@ struct kparam_string {
 extern int parse_args(const char *name,
                       char *args,
                       struct kernel_param *params,
- unsigned num,
+ struct kernel_param *end,
                       int (*unknown)(char *param, char *val));
 
 /* All the helper functions */
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.71-bk1/init/main.c working-2.5.71-bk1-param/init/main.c
--- linux-2.5.71-bk1/init/main.c 2003-06-15 11:30:10.000000000 +1000
+++ working-2.5.71-bk1-param/init/main.c 2003-06-16 10:18:25.000000000 +1000
@@ -404,7 +404,7 @@ asmlinkage void __init start_kernel(void
         page_alloc_init();
         printk("Kernel command line: %s\n", saved_command_line);
         parse_args("Booting kernel", command_line, &__start___param,
- &__stop___param - &__start___param,
+ &__stop___param,
                    &unknown_bootoption);
         trap_init();
         rcu_init();
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.71-bk1/kernel/module.c working-2.5.71-bk1-param/kernel/module.c
--- linux-2.5.71-bk1/kernel/module.c 2003-06-15 11:30:11.000000000 +1000
+++ working-2.5.71-bk1-param/kernel/module.c 2003-06-16 10:19:53.000000000 +1000
@@ -929,7 +929,7 @@ static int obsolete_params(const char *n
                 kp[i].arg = &obsparm[i];
         }
 
- ret = parse_args(name, args, kp, num, NULL);
+ ret = parse_args(name, args, kp, kp + num, NULL);
  out:
         kfree(kp);
         return ret;
@@ -1623,10 +1623,9 @@ static struct module *load_module(void _
         } else {
                 /* Size of section 0 is 0, so this works well if no params */
                 err = parse_args(mod->name, mod->args,
- (struct kernel_param *)
- sechdrs[setupindex].sh_addr,
- sechdrs[setupindex].sh_size
- / sizeof(struct kernel_param),
+ (void *)sechdrs[setupindex].sh_addr,
+ (void *)sechdrs[setupindex].sh_addr
+ + sechdrs[setupindex].sh_size,
                                  NULL);
         }
         if (err < 0)
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.71-bk1/kernel/params.c working-2.5.71-bk1-param/kernel/params.c
--- linux-2.5.71-bk1/kernel/params.c 2003-02-11 14:26:20.000000000 +1100
+++ working-2.5.71-bk1-param/kernel/params.c 2003-06-16 10:18:25.000000000 +1000
@@ -46,13 +46,13 @@ static inline int parameq(const char *in
 static int parse_one(char *param,
                      char *val,
                      struct kernel_param *params,
- unsigned num_params,
+ struct kernel_param *end,
                      int (*handle_unknown)(char *param, char *val))
 {
         unsigned int i;
 
         /* Find parameter */
- for (i = 0; i < num_params; i++) {
+ for (i = 0; &params[i] < end; i++) {
                 if (parameq(param, params[i].name)) {
                         DEBUGP("They are equal! Calling %p\n",
                                params[i].set);
@@ -109,7 +109,7 @@ static char *next_arg(char *args, char *
 int parse_args(const char *name,
                char *args,
                struct kernel_param *params,
- unsigned num,
+ struct kernel_param *end,
                int (*unknown)(char *param, char *val))
 {
         char *param, *val;
@@ -120,7 +120,7 @@ int parse_args(const char *name,
                 int ret;
 
                 args = next_arg(args, &param, &val);
- ret = parse_one(param, val, params, num, unknown);
+ ret = parse_one(param, val, params, end, unknown);
                 switch (ret) {
                 case -ENOENT:
                         printk(KERN_ERR "%s: Unknown parameter `%s'\n", 

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

This archive was generated by hypermail 2b29 : Sun Jun 15 2003 - 22:00:43 EST