| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Sep 2013 15:28:34 +0200
From: Frantisek Hrbata <fhrbata@...hat.com>
To: Rusty Russell <rusty@...tcorp.com.au>
Cc: Kyle McMartin <kyle@...radead.org>, linux-kernel@...r.kernel.org,
jstancek@...hat.com, keescook@...omium.org,
peter.oberparleiter@...ibm.com, linux-arch@...r.kernel.org,
arnd@...db.de, mgahagan@...hat.com, agospoda@...hat.com,
akpm@...ux-foundation.org
Subject: Re: [PATCH v2 4/4] kernel: add support for init_array constructors
On Tue, Sep 10, 2013 at 03:05:57PM +0930, Rusty Russell wrote:
> Frantisek Hrbata <fhrbata@...hat.com> writes:
> > On Mon, Sep 09, 2013 at 10:44:03AM +0930, Rusty Russell wrote:
> >> Kyle McMartin <kyle@...radead.org> 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@...hat.com>
> >> >> >
> >> >> > 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.
Thank you Rusty, from what I can say it looks ok to me. So I would go with this
version. Is there anything that needs to be done to consider this as the
correct version of the 4/4 patch? Meaning should we repost this as v3 or could
your version of the patch be picked as you posted it?
>
> From: Frantisek Hrbata <fhrbata@...hat.com>
> 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@...hat.com>
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
>
> 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)
--
Frantisek Hrbata
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists