linux-kernel - Re: [PATCH v2 bpf-next 4/5] selftests/bpf: Add tests for bpf_cgroup_read

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: <87ecvew7pv.fsf@oracle.com>
Date: Fri, 20 Jun 2025 21:09:32 +0200
From: "Jose E. Marchesi" <jose.marchesi@...cle.com>
To: Eduard Zingerman <eddyz87@...il.com>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Song Liu
 <song@...nel.org>, "Jose E. Marchesi" <jemarch@....org>,
        bpf
 <bpf@...r.kernel.org>,
        Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        LSM List
 <linux-security-module@...r.kernel.org>,
        Kernel Team
 <kernel-team@...a.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Alexei
 Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Alexander Viro
 <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan
 Kara <jack@...e.cz>,
        KP Singh <kpsingh@...nel.org>,
        Matt Bobrowski
 <mattbobrowski@...gle.com>,
        Amir Goldstein <amir73il@...il.com>,
        Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejun Heo <tj@...nel.org>, Daan De Meyer <daan.j.demeyer@...il.com>
Subject: Re: [PATCH v2 bpf-next 4/5] selftests/bpf: Add tests for
 bpf_cgroup_read_xattr



> On Fri, 2025-06-20 at 11:11 -0700, Alexei Starovoitov wrote:
>> On Thu, Jun 19, 2025 at 3:02 PM Song Liu <song@...nel.org> wrote:
>> > +       bpf_dynptr_from_mem(xattr_value, sizeof(xattr_value), 0, &value_ptr);
>> 
>> https://github.com/kernel-patches/bpf/actions/runs/15767046528/job/44445539248
>> 
>> progs/cgroup_read_xattr.c:19:9: error: ‘bpf_dynptr_from_mem’ is static
>> but used in inline function ‘read_xattr’ which is not static [-Werror]
>> 19 | bpf_dynptr_from_mem(value, sizeof(value), 0, &value_ptr);
>> > ^~~~~~~~~~~~~~~~~~~
>> 
>> 
>> Jose,
>> 
>> Could you please help us understand this gcc-bpf error ?
>> What does it mean?
>
> Not Jose, but was curious.
> Some googling lead to the following C99 wording [1]:
>
>   > An inline definition of a function with external linkage shall not
>   > contain a definition of a modifiable object with static storage
>   > duration, and shall not contain a reference to an identifier with
>   > internal linkage
>
> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>     6.7.4 Function specifiers, paragraph 3
>
> The helper is defined as `static`:
>
>   static long (* const bpf_dynptr_from_mem)(...) = (void *) 197;
>
> While `read_xattr` has external linkage:
>
>   __always_inline void read_xattr(struct cgroup *cgroup)
>   {
> 	...
> 	bpf_dynptr_from_mem(value, sizeof(value), 0, &value_ptr);
> 	...
>   }
>
> I think that declaring `read_xattr` as `static` should help with gcc.

Yes I agree that is the issue.  It is a restriction introduced by C99.
I wasn't aware of it so I had to look it up.

If you need read_xattr to be accessible from other compilation units and
inlinable, you may declare it as static inline and put it in a header
file.  It will then use the copies of the helper pointers in the other
compilation units.

Alternatively, with GCC you could try to add a direct declaration for
the helper function to progs/cgroup_read_xattr.c, that doesnt need to
use any static intermediate pointer:

  #if COMPILING_WITH_GCC
  long _bpf_dynptr_from_mem (...) __attribute__ ((kernel_helper (197)));
  #define bpf_dynptr_from_mem _bpf_dynptr_from_mem
  #endif