strace for m68k bpf_prog_info mismatch

[Baruch Siach]

Hi Geert,

On Fri, May 03 2019, Geert Uytterhoeven wrote:
> On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch at tkos.co.il> wrote:
>> strace 5.0 fails to build for m86k/5208 with the Buildroot generated
>> toolchain:
>>
>> In file included from bpf_attr_check.c:6:0:
>> static_assert.h:20:25: error: static assertion failed: "bpf_prog_info_struct.nr_jited_ksyms offset mismatch"
>>  #  define static_assert _Static_assert
>>                          ^
>> bpf_attr_check.c:913:2: note: in expansion of macro ‘static_assert’
>>   static_assert(offsetof(struct bpf_prog_info_struct, nr_jited_ksyms) == offsetof(struct bpf_prog_info, nr_jited_ksyms),
>>   ^~~~~~~~~~~~~
>>
>> The direct cause is a difference in the hole after the gpl_compatible
>> field. Here is pahole output for the kernel struct (from v4.19):
>>
>> struct bpf_prog_info {
>>         ...
>>         __u32                      ifindex;              /*    80     4 */
>>         __u32                      gpl_compatible:1;     /*    84: 0  4 */
>>
>>         /* XXX 15 bits hole, try to pack */
>>         /* Bitfield combined with next fields */
>>
>>         __u64                      netns_dev;            /*    86     8 */
>
> I guess that should be "__aligned_u64 netns_dev;", to not rely on
> implicit alignment.

Thanks. I can confirm that this minimal change fixes strace build:

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 929c8e537a14..709d4dddc229 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2869,7 +2869,7 @@ struct bpf_prog_info {
 	char name[BPF_OBJ_NAME_LEN];
 	__u32 ifindex;
 	__u32 gpl_compatible:1;
-	__u64 netns_dev;
+	__aligned_u64 netns_dev;
 	__u64 netns_ino;
 	__u32 nr_jited_ksyms;
 	__u32 nr_jited_func_lens;

Won't that break ABI compatibility for affected architectures?

>> And this is for the strace struct:
>>
>> struct bpf_prog_info_struct {
>>         ...
>>         uint32_t                   ifindex;              /*    80     4 */
>>         uint32_t                   gpl_compatible:1;     /*    84: 0  4 */
>>
>>         /* XXX 31 bits hole, try to pack */
>
> How come the uint64_t below is 8-byte aligned, not 2-byte aligned?
> Does strace use a special definition of uint64_t?

I guess this is because of the netns_dev field definition in struct
bpf_prog_info_struct at bpf_attr.h:

struct bpf_prog_info_struct {
       ...
        uint32_t gpl_compatible:1;
        /*
         * The kernel UAPI is broken by Linux commit
         * v4.16-rc1~123^2~227^2~5^2~2 .
         */
        uint64_t ATTRIBUTE_ALIGNED(8) netns_dev; /* skip check */

Alignment is forced.

>>         uint64_t                   netns_dev;            /*    88     8 */
>>
>> How should this be fixed?
>
> IMHO all "__u64" in structs tagged "__attribute__((aligned(8)))" should
> be replaced by "__aligned_u64", which is what the (whitespace-damaged)
> diff below does.
>
> Note that changing __bpf_md_ptr() may have a visible effect, as it is
> used with struct bpf_sock, which has a size that is not a multiple of
> 8 bytes.  So depending on architecture, a hole may appear.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -