[LTP] [PATCH] syscalls/bpf: zero-initialize bpf_attr including padding bits

[Jan Stancek]

On Thu, Feb 6, 2025 at 3:08 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > However kernel bpf syscall checks that all unused fields for a command
> > are set to zero in CHECK_ATTR() macro, which causes tests to fail with
> > EINVAL.
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >  testcases/kernel/syscalls/bpf/bpf_common.c | 32 ++++++++++++----------
> >  1 file changed, 18 insertions(+), 14 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
> > index 95b5bc12eaa4..d765c4e32936 100644
> > --- a/testcases/kernel/syscalls/bpf/bpf_common.c
> > +++ b/testcases/kernel/syscalls/bpf/bpf_common.c
> > @@ -49,13 +49,14 @@ int bpf_map_create(union bpf_attr *const attr)
> >
> >  int bpf_map_array_create(const uint32_t max_entries)
> >  {
> > -     union bpf_attr map_attr = {
> > -             .map_type = BPF_MAP_TYPE_ARRAY,
> > -             .key_size = 4,
> > -             .value_size = 8,
> > -             .max_entries = max_entries,
> > -             .map_flags = 0
> > -     };
> > +     /* zero-initialize entire struct including padding bits */
> > +     union bpf_attr map_attr = {};
> > +
> > +     map_attr.map_type = BPF_MAP_TYPE_ARRAY;
> > +     map_attr.key_size = 4;
> > +     map_attr.value_size = 8;
> > +     map_attr.max_entries = max_entries;
> > +     map_attr.map_flags = 0;
>
> I had a closer look here, the map_attr is an union with anonymous
> structures and I suppose that the problem here is that the padding after
> the union is no longer cleared and that there have been some new fields
> added, at least compared to the lapi fallback structures we have and we
> possibly pass random mess in flags.

It's not just padding, some fields (from other structs) are also not
initialized:

void bpf_map_array_get(const int map_fd,
                       const uint32_t *const array_indx,
                       uint64_t *const array_val)
{
        union bpf_attr elem_attr = {
                .map_fd = map_fd,
                .key = ptr_to_u64(array_indx),
                .value = ptr_to_u64(array_val),
                .flags = 0
        };

        printf("should be zero? %u\n", elem_attr.func_info_cnt);

and I get:
should be zero? 4202093

func_info_cnt is at offset 88, which is far away from ones we
initialized explicitly:

        struct {
                uint32_t           map_fd;             /*     0     4 */

                /* XXX 4 bytes hole, try to pack */

                uint64_t           key
__attribute__((__aligned__(8))); /*     8     8 */
                union {
                        uint64_t   value
__attribute__((__aligned__(8))); /*    16     8 */
                        uint64_t   next_key
__attribute__((__aligned__(8))); /*    16     8 */
                } __attribute__((__aligned__(8)));     /*    16     8 */
                uint64_t           flags;              /*    24     8 */
        } __attribute__((__aligned__(8)))
__attribute__((__aligned__(8)));             /*     0    32 */

>
> Maybe slightly better version would be:
>
> diff --git a/testcases/kernel/syscalls/bpf/bpf_common.c b/testcases/kernel/syscalls/bpf/bpf_common.c
> index 95b5bc12e..a8289e106 100644
> --- a/testcases/kernel/syscalls/bpf/bpf_common.c
> +++ b/testcases/kernel/syscalls/bpf/bpf_common.c
> @@ -49,7 +49,9 @@ int bpf_map_create(union bpf_attr *const attr)
>
>  int bpf_map_array_create(const uint32_t max_entries)
>  {
> -       union bpf_attr map_attr = {
> +       union bpf_attr map_attr = {};
> +
> +       map_attr = (union bpf_attr) {
>                 .map_type = BPF_MAP_TYPE_ARRAY,
>                 .key_size = 4,
>                 .value_size = 8,
>
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>