linux-kernel - Re: [PATCH] jump_label: align jump

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170227160601.5b79a1fe@gandalf.local.home>
Date:   Mon, 27 Feb 2017 16:06:01 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     David Daney <ddaney@...iumnetworks.com>
Cc:     Jason Baron <jbaron@...mai.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mips@...ux-mips.org, linuxppc-dev@...ts.ozlabs.org,
        Ingo Molnar <mingo@...nel.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Anton Blanchard <anton@...ba.org>,
        Rabin Vincent <rabin@....in>,
        Russell King <linux@...linux.org.uk>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Chris Metcalf <cmetcalf@...lanox.com>,
        Zhigang Lu <zlu@...hip.com>
Subject: Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes

On Mon, 27 Feb 2017 11:59:50 -0800
David Daney <ddaney@...iumnetworks.com> wrote:

> For me the size is not the important issue, it is the alignment of the 
> struct jump_entry entries in the table.  I don't understand how your 
> patch helps, and I cannot Acked-by unless I understand what is being 
> done and can see that it is both correct and necessary.

You brought up a very good point and I'm glad that I had Jason Cc all
the arch maintainers in one patch.

I think jump_labels may be much more broken than we think, and Jason's
fix doesn't fix anything. We had this same issues with tracepoints.

I'm looking at jump_label_init, and how we iterate over an array of
struct jump_entry's that was put together by the linker. The problem is
that jump_entry is not a power of 2 in size.

struct jump_entry {
	jump_label_t code;
	jump_label_t target;
	jump_label_t key;
};

When putting together arrays of this kind, the linker is in its right
to add padding for alignment, in the middle of the array! It has no
idea that this is an array, and there's nothing stopping the linker
from messing it up.

For those structs that are a power of 2 in size, there's no reason for
the linker to do anything else, and it "just works". There's plenty of
instances in the kernel that depend on this.

I'm thinking that the sort algorithm either hid the problem or fixed it
somehow (I'm guessing it hid the problem).

I hit the same issue with trace event structures. The solution was to
create the array of pointers to each structure, and dereference the
structures from the array.

See commit e4a9ea5ee ("tracing: Replace trace_event struct array with
pointer array")

-- Steve