netdev - Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL_

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

Message-ID: <20181129021635.rzmehzzfgt2jvpgi@bytedancedeMacBook-Air.local>
Date:   Thu, 29 Nov 2018 10:16:35 +0800
From:   Xiaozhou Liu <lxz1983@...il.com>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        ast@...nel.org, daniel@...earbox.net, yhs@...com,
        jakub.kicinski@...ronome.com, quentin.monnet@...ronome.com
Subject: Re: [PATCH] Compiler Attributes: move kernel-only attributes into
 __KERNEL__

Hi Miguel,

On Wed, Nov 28, 2018 at 06:35:18PM +0100, Miguel Ojeda wrote:
> Hi Xiaozhou,
> 
> On Wed, Nov 28, 2018 at 3:09 PM Xiaozhou Liu <liuxiaozhou@...edance.com> wrote:
> >
> > Attributes such as `__gnu_inline' are meant to be used within the
> > kernel. When userspace somehow includes <linux/compiler.h>
> > (eg. tools/bpf), compilation errors would be shown:
> >
> >         "error: unknown type name ‘__gnu_inline’"
> >
> > So just move these things into __KERNEL__ and the behavior is kept
> > as before.

By `these' I mean inline and the like, to be clear.

> That is not exactly correct -- a3f8a30f3f00 moved some attributes to
> another file, moving them into __KERNEL__ (in particular,__gnu_inline
> is).

Yes, that is what a3f8a30f3f00 did. Sorry.
Turns out the commits in question are 815f0ddb346c and a3f8a30f3f00.

> The problem is, instead, that __gnu_inline is not anymore defined
> outside __KERNEL__, but something else that uses it is (the inline
> macro definition, if I had to guess).

Yes and no. Let's recall the whole story.

Before 815f0ddb346c("include/linux/compiler*.h: make compiler-*.h mutually exclusive"),
__gnu_inline and inline were both *in* __KERNEL__ as the were in
<linux/compiler-gcc.h>, which was entirely put to __KERNEL__ in
<linux/compiler_types.h>. Everything was fine.

Then 815f0ddb346c moved inline and __gnu_inline *out* of __KERNEL__
and put them in <linux/compiler_types.h> so userspace could see them
both. Not sure if it's intended behavior, but everything looked fine.

Then a3f8a30f3f00 moved __gnu_inline back into __KERNEL__ and left
inline behind. Since inline depends on __gnu_inline, error showing
"unknown type name ‘__gnu_inline’" pops up.

> If your problem is fixed by putting __gnu_inline into __KERNEL__
> again, it means we can simply move the inline definition inside
> __KERNEL__ too. That way, we don't pollute userspace users with macro
> definitions.

It'd be good. That's exactly what my patch does.

> Having said that, does someone know whether userspace should have
> access to those attributes (or rather, other code that uses in turn
> those attributes)?

It'd be better to keep those attributes out of userspace, as is the
case before 815f0ddb346c.

BTW, the userspace code failed to compile in my case is under
directory tools/bpf/.

Thanks,
Xiaozhou