Re: [PATCH v2 4/4] kernel: add support for init_array constructors

From: Rusty Russell
Date: Tue Sep 10 2013 - 07:48:04 EST


Frantisek Hrbata <fhrbata@xxxxxxxxxx> writes:
> On Mon, Sep 09, 2013 at 10:44:03AM +0930, Rusty Russell wrote:
>> Kyle McMartin <kyle@xxxxxxxxxxxxx> writes:
>> > On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
>> >> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
>> >> > > .ctors or .init_array, but not both at the same time
>> >> > >
>> >> > > Signed-off-by: Frantisek Hrbata <fhrbata@xxxxxxxxxx>
>> >> >
>> >> > Might be nice to document which gcc version changed this, so people can
>> >> > choose whether to cherry-pick this change?
>> >>
>> >> Thank you for pointing this out. As per gcc git this was introduced by commit
>> >> ef1da80 and released in 4.7 version.
>> >>
>> >> $ git describe --contains ef1da80
>> >> gcc-4_7_0-release~4358
>> >>
>> >> Do you want me to post v3 with this info included in the descrition?
>> >>
>> >
>> > It actually depends on the combination of binutils/ld and gcc you use, not
>> > simply which gcc version you use. :/
>>
>> Indeed, and seems it was binutils 20110507 which actually handled it
>> properly.
>>
>> AFAICT it's theoretically possible to have .ctors and .init_array in a
>> module. Unlikely, but the patch should check for both and refuse to
>> load the module in that case. Otherwise weird things would happen.
>
> I'm not sure if coexistence of .ctors and .init_array sections should result in
> denial of module, but I for sure know nothing about this :). Could you maybe
> privide one example of the "weird thing"?

Well, if we have both ctors and init_array, and we only call the ctors,
part of the module will be uninitialized.

I was thinking about something like the following (based on your
previous patch).

Thoughts?
Rusty.

From: Frantisek Hrbata <fhrbata@xxxxxxxxxx>
Subject: kernel: add support for init_array constructors

This adds the .init_array section as yet another section with constructors. This
is needed because gcc could add __gcov_init calls to .init_array or .ctors
section, depending on gcc (and binutils) version .

v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
.ctors or .init_array, but not both at the same time
v3: - fail to load if that does happen somehow.

Signed-off-by: Frantisek Hrbata <fhrbata@xxxxxxxxxx>
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 83e2c31..bc2121f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -473,6 +473,7 @@
#define KERNEL_CTORS() . = ALIGN(8); \
VMLINUX_SYMBOL(__ctors_start) = .; \
*(.ctors) \
+ *(.init_array) \
VMLINUX_SYMBOL(__ctors_end) = .;
#else
#define KERNEL_CTORS()
diff --git a/kernel/module.c b/kernel/module.c
index dc58274..d3f5a58 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2738,7 +2738,7 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
return 0;
}

-static void find_module_sections(struct module *mod, struct load_info *info)
+static int find_module_sections(struct module *mod, struct load_info *info)
{
mod->kp = section_objs(info, "__param",
sizeof(*mod->kp), &mod->num_kp);
@@ -2768,6 +2768,18 @@ static void find_module_sections(struct module *mod, struct load_info *info)
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
sizeof(*mod->ctors), &mod->num_ctors);
+ if (!mod->ctors)
+ mod->ctors = section_objs(info, ".init_array",
+ sizeof(*mod->ctors), &mod->num_ctors);
+ else if (find_sec(info, ".init_array")) {
+ /*
+ * This shouldn't happen with same compiler and binutils
+ * building all parts of the module.
+ */
+ printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
+ mod->name);
+ return -EINVAL;
+ }
#endif

#ifdef CONFIG_TRACEPOINTS
@@ -2806,6 +2818,8 @@ static void find_module_sections(struct module *mod, struct load_info *info)

info->debug = section_objs(info, "__verbose",
sizeof(*info->debug), &info->num_debug);
+
+ return 0;
}

static int move_module(struct module *mod, struct load_info *info)
@@ -3263,7 +3277,9 @@ static int load_module(struct load_info *info, const char __user *uargs,

/* Now we've got everything in the final locations, we can
* find optional sections. */
- find_module_sections(mod, info);
+ err = find_module_sections(mod, info);
+ if (err)
+ goto free_unload;

err = check_module_license_and_versions(mod);
if (err)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/