linux-kernel - Re: [PATCH v5] earlycon: Use a pointer table to fix __earlycon

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] [day] [month] [year] [list]

Message-ID: <20180326222421.bwkdakaofoixp562@rob-hp-laptop>
Date:   Mon, 26 Mar 2018 17:24:21 -0500
From:   Rob Herring <robh@...nel.org>
To:     Daniel Kurtz <djkurtz@...omium.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Guenter Roeck <groeck@...omium.org>, adurbin@...omium.org,
        linux-kernel@...r.kernel.org,
        Frank Rowand <frowand.list@...il.com>,
        Jiri Slaby <jslaby@...e.com>, Arnd Bergmann <arnd@...db.de>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE" 
        <devicetree@...r.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>,
        "open list:GENERIC INCLUDE/ASM HEADER FILES" 
        <linux-arch@...r.kernel.org>
Subject: Re: [PATCH v5] earlycon: Use a pointer table to fix __earlycon_table
 stride

On Tue, Mar 20, 2018 at 04:05:36PM -0600, Daniel Kurtz wrote:
> Commit 99492c39f39f ("earlycon: Fix __earlycon_table stride") tried to fix
> __earlycon_table stride by forcing the earlycon_id struct alignment to 32
> and asking the linker to 32-byte align the __earlycon_table symbol.  This
> fix was based on commit 07fca0e57fca92 ("tracing: Properly align linker
> defined symbols") which tried a similar fix for the tracing subsystem.
> 
> However, this fix doesn't quite work because there is no guarantee that
> gcc will place structures packed into an array format.  In fact, gcc 4.9
> chooses to 64-byte align these structs by inserting additional padding
> between the entries because it has no clue that they are supposed to be in
> an array.  If we are unlucky, the linker will assign symbol
> "__earlycon_table" to a 32-byte aligned address which does not correspond
> to the 64-byte aligned contents of section "__earlycon_table".
> 
> To address this same problem, the fix to the tracing system was
> subsequently re-implemented using a more robust table of pointers approach
> by commits:
>  3d56e331b653 ("tracing: Replace syscall_meta_data struct array with pointer array")
>  654986462939 ("tracepoints: Fix section alignment using pointer array")
>  e4a9ea5ee7c8 ("tracing: Replace trace_event struct array with pointer array")
> 
> Let's use this same "array of pointers to structs" approach for
> EARLYCON_TABLE.

Wouldn't every use of linker sections with structs have this problem? We 
use them for clocks, irqs, timers unless those have been fixed.

> 
> Fixes: 99492c39f39f ("earlycon: Fix __earlycon_table stride")
> Signed-off-by: Daniel Kurtz <djkurtz@...omium.org>
> Suggested-by: Aaron Durbin <adurbin@...omium.org>
> ---
> Changes since v2:
>  * Use __initconst instead of __initdata to avoid h8300 and alpha kbuild errors
> 
> Changes since v3:
>  * Fixed typos in commit message
> 
> Changes since v4:
>  * removed Change-Id: from commit message
> 
>  drivers/of/fdt.c                  |  7 +++++--
>  drivers/tty/serial/earlycon.c     |  6 ++++--
>  include/asm-generic/vmlinux.lds.h |  2 +-
>  include/linux/serial_core.h       | 21 ++++++++++++++-------
>  4 files changed, 24 insertions(+), 12 deletions(-)

Reviewed-by: Rob Herring <robh@...nel.org>