[PATCH net] bpf: enforce correct alignment for instructions

[PATCH net] bpf: enforce correct alignment for instructions

[Eric Dumazet]

After commit 9facc336876f ("bpf: reject any prog that failed read-only lock")
offsetof(struct bpf_binary_header, image) became 3 instead of 4,
breaking powerpc BPF badly, since instructions need to be word aligned.

Fixes: 9facc336876f ("bpf: reject any prog that failed read-only lock")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/filter.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index b615df57b7d5b2ccb468c411c3a2aae103cd2aea..20f2659dd829256d7fef206087ab3262e1e291f5 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -472,7 +472,9 @@ struct sock_fprog_kern {
 struct bpf_binary_header {
 	u16 pages;
 	u16 locked:1;
-	u8 image[];
+
+	/* Some arches need word alignment for their instructions */
+	u8 image[] __aligned(4);
 };
 
 struct bpf_prog {
-- 
2.18.0.rc1.244.gcf134e6275-goog

Re: [PATCH net] bpf: enforce correct alignment for instructions

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Wed, 20 Jun 2018 17:24:09 -0700

> After commit 9facc336876f ("bpf: reject any prog that failed read-only lock")
> offsetof(struct bpf_binary_header, image) became 3 instead of 4,
> breaking powerpc BPF badly, since instructions need to be word aligned.
> 
> Fixes: 9facc336876f ("bpf: reject any prog that failed read-only lock")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I'll apply this directly, thanks Eric.

Re: [PATCH net] bpf: enforce correct alignment for instructions

[Eric Dumazet]

On 06/20/2018 08:46 PM, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Wed, 20 Jun 2018 17:24:09 -0700
> 
>> After commit 9facc336876f ("bpf: reject any prog that failed read-only lock")
>> offsetof(struct bpf_binary_header, image) became 3 instead of 4,
>> breaking powerpc BPF badly, since instructions need to be word aligned.
>>
>> Fixes: 9facc336876f ("bpf: reject any prog that failed read-only lock")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> I'll apply this directly, thanks Eric.
> 

Thanks David :)

Re: [PATCH net] bpf: enforce correct alignment for instructions

[Daniel Borkmann]

On 06/21/2018 06:08 AM, Eric Dumazet wrote:
> On 06/20/2018 08:46 PM, David Miller wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Wed, 20 Jun 2018 17:24:09 -0700
>>
>>> After commit 9facc336876f ("bpf: reject any prog that failed read-only lock")
>>> offsetof(struct bpf_binary_header, image) became 3 instead of 4,
>>> breaking powerpc BPF badly, since instructions need to be word aligned.
>>>
>>> Fixes: 9facc336876f ("bpf: reject any prog that failed read-only lock")
>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>
>> I'll apply this directly, thanks Eric.
> 
> Thanks David :)

Sigh, sorry for the breakage, looks like I got fooled by x86 gcc.

struct bpf_binary_header {
        u16                        pages;                /*     0     2 */
        u16                        locked:1;             /*     2:15  2 */

        /* XXX 15 bits hole, try to pack */

        u8                         image[0];             /*     4     0 */

        /* size: 4, cachelines: 1, members: 3 */
        /* bit holes: 1, sum bit holes: 15 bits */
        /* last cacheline: 4 bytes */
};

Thanks Eric!

Re: [PATCH net] bpf: enforce correct alignment for instructions

[David Miller]

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 21 Jun 2018 23:03:09 +0200

> On 06/21/2018 06:08 AM, Eric Dumazet wrote:
>> On 06/20/2018 08:46 PM, David Miller wrote:
>>> From: Eric Dumazet <edumazet@google.com>
>>> Date: Wed, 20 Jun 2018 17:24:09 -0700
>>>
>>>> After commit 9facc336876f ("bpf: reject any prog that failed read-only lock")
>>>> offsetof(struct bpf_binary_header, image) became 3 instead of 4,
>>>> breaking powerpc BPF badly, since instructions need to be word aligned.
>>>>
>>>> Fixes: 9facc336876f ("bpf: reject any prog that failed read-only lock")
>>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>>
>>> I'll apply this directly, thanks Eric.
>> 
>> Thanks David :)
> 
> Sigh, sorry for the breakage, looks like I got fooled by x86 gcc.
> 
> struct bpf_binary_header {
>         u16                        pages;                /*     0     2 */
>         u16                        locked:1;             /*     2:15  2 */

Note that you can also just make locked a plan u16 for now until you
need more flag bits, the code generated will be more efficient
especially on non-x86.