linux-kernel - RE: [PATCH v2] x86/fpu: use _Alignof to avoid UB in 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: <20220928112341.2255320-1-me@inclyc.cn>
Date:   Wed, 28 Sep 2022 19:23:41 +0800
From:   YingChi Long <me@...lyc.cn>
To:     david.laight@...lab.com
Cc:     bp@...en8.de, chang.seok.bae@...el.com,
        dave.hansen@...ux.intel.com, hpa@...or.com,
        linux-kernel@...r.kernel.org, me@...lyc.cn, mingo@...hat.com,
        ndesaulniers@...gle.com, pbonzini@...hat.com, tglx@...utronix.de,
        x86@...nel.org
Subject: RE: [PATCH v2] x86/fpu: use _Alignof to avoid UB in TYPE_ALIGN

> From: YingChi Long
> > Sent: 27 September 2022 17:44
> >
> > > Interesting - what justification do they give?
> > > Linux kernel requires that the compiler add no unnecessary padding
> > > so that structure definitions are well defined.
> >
> > Yes, that's a clarification given in 2019.
> >
> > > So using a type definition inside offsetof() won't give a
> > > useful value - but it still isn't really UB.
> >
> > WG14 may worry about commas and the scope of new definitions. So they provide
> > new words into the standard and said:
> >
> > > If the specified type defines a new type or if the specified member is a
> > > bit-field, the behavior is undefined.
> >
> > https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm
>
> Except that the kernel requires it to be defined.
>
> Did they clarify the clause that required offsetof() to return
> a compile-time constant?
> That stops you doing offsetof(struct x, member->array[expression]).
> (Oh and the compiler for a common OS disallows any version of that
> even when expression in an integer constant!)

WG14 N2350 may just not require implementation offsetof() accepts any type
definitions within the first param of (not the second), and no further changes
about whether it is compile-time constant?

https://godbolt.org/z/9GsEPnPd6

    #include <stdio.h>

    struct foo {
        int a;
        int b[100];
    };

    int main() {
        int i;
        scanf("%d", &i);
        printf("%d\n", __builtin_offsetof(struct foo, b[i]));
    }

We consider reject type definitions within the first parameter in clang.

For example

    offsetof(struct { int a, b;}, a)

However

    struct foo {
        int a;
        int b[20];
    }
    offsetof(struct foo, b[sizeof(struct { int a, b;})])

Shall be fine.

> >
> > I've provided this link in the patch.
> >
> > > Has that ever worked?
> > > Given:
> > > 	struct foo {
> > > 		int a;
> > > 		char b;
> > > 		char c;
> > > 	};
> >
> > TYPE_ALIGN(struct foo) evaluates to 4 in the previous approach (based on
> > offsetof). _Align(struct foo) evaluates to the same value.
> >
> > See https://godbolt.org/z/sqebhEnsq
> >
> > > I think CHECK_MEMBER_AT_END_OF_TYPE(struct foo, b) is true.
> >
> > Hmm, both the previous version and after this patch the macro gives me
> > false. (See the godbolt link).
>
> See https://godbolt.org/z/95shMx44j
>
> It return 1 for a and 0 for b and c (and char d,e following b).
> NFI what it is trying to do!

Switch _Alignof back to TYPE_ALIGN, CME seems return exactly the same values. I
don't know what CME do here, but seems TYPE_ALIGN and _Alignof have the same
semantics here?

https://godbolt.org/z/hYcT1M3ed