netdev - Re: [PATCH 1/6] perf: Add new type PERF_TYPE

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: <adfbe341-7599-6819-27ef-4bc13cb65d6d@fb.com>
Date:   Thu, 23 Nov 2017 22:31:29 -0800
From:   Alexei Starovoitov <ast@...com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Song Liu <songliubraving@...com>
CC:     <rostedt@...dmis.org>, <mingo@...hat.com>, <davem@...emloft.net>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <daniel@...earbox.net>, <kernel-team@...com>
Subject: Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

On 11/23/17 2:02 AM, Peter Zijlstra wrote:
> On Wed, Nov 15, 2017 at 09:23:33AM -0800, Song Liu wrote:
>
>> Note: We use type __u64 for pointer probe_desc instead of __aligned_u64.
>> The reason here is to avoid changing the size of struct perf_event_attr,
>> and breaking new-kernel-old-utility scenario. To avoid alignment problem
>> with the pointer, we will (in the following patches) copy probe_desc to
>> __aligned_u64 before using it as pointer.
>
> ISTR there are only relatively few architectures where __u64 and
> __aligned_u64 are not the same thing.
>
> The comment that goes with it seems to suggest i386 has short alignment
> for u64 but my compiler says differently:
>
>         printf("%d, %d\n", sizeof(unsigned long long), __alignof__(unsigned long long));
>
> $ gcc -m32 -o align align.c && ./align
> 8, 8

unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include <stdio.h>

struct S {
   unsigned long long a;
} s;

struct U {
   unsigned long long a;
} u;

int main()
{
         printf("%d, %d\n", sizeof(unsigned long long),
                __alignof__(unsigned long long));
         printf("%d, %d\n", sizeof(s), __alignof__(s));
         printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4

so we have to use __aligned_u64 in uapi.

Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.
If you prefer we can do the same around 'config1', but it's not
any prettier.
We considered adding __aligned_u64 to the end of
'struct perf_event_attr', but it's a waste for most users, so reusing
the space of 'config' field like this seems the least evil.