linux-kernel - Re: [PATCH 1/3] module: deal with alignment issues in built-in module versions

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTinFgma3F7ETwC8Mho7wdDK4V2b+Ao__npJNAW8j@mail.gmail.com>
Date:	Mon, 21 Feb 2011 08:38:46 +0100
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Rusty Russell <rusty@...tcorp.com.au>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	David Miller <davem@...emloft.net>, dtor@...are.com,
	linux-kernel@...r.kernel.org, linux-m68k@...r.kernel.org,
	linux-arch@...r.kernel.org
Subject: Re: [PATCH 1/3] module: deal with alignment issues in built-in module versions

On Mon, Feb 21, 2011 at 05:00, Rusty Russell <rusty@...tcorp.com.au> wrote:
>> Except that .long is 32-bit on ppc64 :-( You need .llong for 64-bit.
>
> OK, all options suck.  Do we want the workaround or not?

We can discuss about that until someone gets bitten by that.

But please fix the "aligned(sizeof(void *))"-in-one-place-only issue.

> Cheers,
> Rusty.
>
> Subject: module: deal with alignment issues in built-in module versions
> Date: Mon, 7 Feb 2011 16:02:25 -0800
> From: Dmitry Torokhov <dtor@...are.com>
>
> On m68k natural alignment is 2-byte boundary but we are trying to
> align structures in __modver section on sizeof(void *) boundary.
> This causes trouble when we try to access elements in this section
> in array-like fashion when create "version" attributes for built-in
> modules.
>
> Moreover, as DaveM said, we can't reliably put structures into
> independent objects, put them into a special section, and then expect
> array access over them (via the section boundaries) after linking the
> objects together to just "work" due to variable alignment choices in
> different situations. The only solution that seems to work reliably
> is to make an array of plain pointers to the objects in question and
> put those pointers in the special section.
>
> Reported-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> Signed-off-by: Dmitry Torokhov <dtor@...are.com>
> Signed-off-by: Rusty Russell <rusty@...tcorp.com.au>
> ---
>  include/linux/module.h |   10 +++++-----
>  kernel/params.c        |    9 ++++++---
>  2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 9bdf27c..9b7081a 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -174,10 +174,7 @@ extern struct module __this_module;
>  #define MODULE_VERSION(_version)                                       \
>        extern ssize_t __modver_version_show(struct module_attribute *, \
>                                             struct module *, char *);  \
> -       static struct module_version_attribute __modver_version_attr    \
> -       __used                                                          \
> -    __attribute__ ((__section__ ("__modver"),aligned(sizeof(void *)))) \
> -       = {                                                             \
> +       static struct module_version_attribute ___modver_attr = {       \
>                .mattr  = {                                             \
>                        .attr   = {                                     \
>                                .name   = "version",                    \
> @@ -187,7 +184,10 @@ extern struct module __this_module;
>                },                                                      \
>                .module_name    = KBUILD_MODNAME,                       \
>                .version        = _version,                             \
> -       }
> +       };                                                              \
> +       static const struct module_version_attribute                    \
> +       __used __attribute__ ((__section__ ("__modver")))               \
> +       * __moduleparam_const __modver_attr = &___modver_attr
>  #endif
>
>  /* Optional firmware file (or files) needed by the module
> diff --git a/kernel/params.c b/kernel/params.c
> index 0da1411..09a08cb 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -821,15 +821,18 @@ ssize_t __modver_version_show(struct module_attribute *mattr,
>        return sprintf(buf, "%s\n", vattr->version);
>  }
>
> -extern struct module_version_attribute __start___modver[], __stop___modver[];
> +extern const struct module_version_attribute *__start___modver[];
> +extern const struct module_version_attribute *__stop___modver[];
>
>  static void __init version_sysfs_builtin(void)
>  {
> -       const struct module_version_attribute *vattr;
> +       const struct module_version_attribute **p;
>        struct module_kobject *mk;
>        int err;
>
> -       for (vattr = __start___modver; vattr < __stop___modver; vattr++) {
> +       for (p = __start___modver; p < __stop___modver; p++) {
> +               const struct module_version_attribute *vattr = *p;
> +
>                mk = locate_module_kobject(vattr->module_name);
>                if (mk) {
>                        err = sysfs_create_file(&mk->kobj, &vattr->mattr.attr);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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/