diff mbox series

[bpf-next,2/2] selftests/bpf: Ensure libbpf skip all-zeros fields of struct_ops maps.

Message ID 20240312183245.341141-3-thinker.li@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Ignore additional fields in the struct_ops maps in an updated version. | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 11 maintainers not CCed: shuah@kernel.org daniel@iogearbox.net kpsingh@kernel.org eddyz87@gmail.com haoluo@google.com linux-kselftest@vger.kernel.org sdf@google.com yonghong.song@linux.dev jolsa@kernel.org john.fastabend@gmail.com mykolal@fb.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 60 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 fail Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 fail Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 fail Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 fail Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 fail Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 fail Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 fail Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-39 fail Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 fail Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc

Commit Message

Kui-Feng Lee March 12, 2024, 6:32 p.m. UTC 
A new version of a type may have additional fields that do not exist in
older versions. Previously, libbpf would reject struct_ops maps with a new
version containing extra fields when running on a machine with an old
kernel. However, we have updated libbpf to ignore these fields if their
values are all zeros or null in order to provide backward compatibility.

Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
 .../bpf/prog_tests/test_struct_ops_module.c   | 35 +++++++++++++++++++
 .../selftests/bpf/progs/struct_ops_module.c   | 13 +++++++
 2 files changed, 48 insertions(+)

Comments

Andrii Nakryiko March 12, 2024, 11:10 p.m. UTC | #1 
On Tue, Mar 12, 2024 at 11:32 AM Kui-Feng Lee <thinker.li@gmail.com> wrote:
>
> A new version of a type may have additional fields that do not exist in
> older versions. Previously, libbpf would reject struct_ops maps with a new
> version containing extra fields when running on a machine with an old
> kernel. However, we have updated libbpf to ignore these fields if their
> values are all zeros or null in order to provide backward compatibility.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>  .../bpf/prog_tests/test_struct_ops_module.c   | 35 +++++++++++++++++++
>  .../selftests/bpf/progs/struct_ops_module.c   | 13 +++++++
>  2 files changed, 48 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> index ee5372c7f2c7..e0d9ff75121b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> @@ -93,9 +93,44 @@ static void test_struct_ops_load(void)
>         struct_ops_module__destroy(skel);
>  }
>
> +static void test_struct_ops_not_zeroed(void)
> +{
> +       struct struct_ops_module *skel;
> +       int err;
> +
> +       skel = struct_ops_module__open();
> +       if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
> +               return;
> +
> +       bpf_program__set_autoload(skel->progs.test_3, false);

maybe mark test_3 program in progs/struct_ops_module.c as default
not-autoload (SEC("?struct_ops/test_3")), so you don't have to set
this to false everywhere?

> +
> +       err = struct_ops_module__load(skel);
> +       struct_ops_module__destroy(skel);
> +
> +       if (!ASSERT_OK(err, "struct_ops_module_load"))
> +               return;
> +
> +       skel = struct_ops_module__open();
> +       if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_not_zeroed"))
> +               return;
> +
> +       bpf_program__set_autoload(skel->progs.test_3, false);
> +
> +       /* libbpf should reject the testmod_zeroed since the value of its
> +        * "zeroed" is non-zero.
> +        */
> +       skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef;
> +       err = struct_ops_module__load(skel);
> +       ASSERT_ERR(err, "struct_ops_module_load_not_zeroed");
> +
> +       struct_ops_module__destroy(skel);
> +}
> +
>  void serial_test_struct_ops_module(void)
>  {
>         if (test__start_subtest("test_struct_ops_load"))
>                 test_struct_ops_load();
> +       if (test__start_subtest("test_struct_ops_not_zeroed"))
> +               test_struct_ops_not_zeroed();
>  }
>
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
> index 026cabfa7f1f..d7d7606f639c 100644
> --- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
> @@ -54,3 +54,16 @@ struct bpf_testmod_ops___v2 testmod_2 = {
>         .test_1 = (void *)test_1,
>         .test_2 = (void *)test_2_v2,
>  };
> +
> +struct bpf_testmod_ops___zeroed {
> +       int (*test_1)(void);
> +       void (*test_2)(int a, int b);
> +       int (*test_maybe_null)(int dummy, struct task_struct *task);
> +       int zeroed;
> +};
> +
> +SEC(".struct_ops.link")
> +struct bpf_testmod_ops___zeroed testmod_zeroed = {
> +       .test_1 = (void *)test_1,
> +       .test_2 = (void *)test_2_v2,
> +};

We should also test the case where we have local ops definition with
incompatible callback function signature (e.g., extra argument or
something). Test then would update the program pointer (through
skeleton's shadow type) to a compatible implementation.

Can you please add a test like that? Because the goal is to have a
single struct_ops definition, if possible, and adjust it at runtime to
make it compatible with the old kernel, let's have a small demo of
this working.

> --
> 2.34.1
>
Kui-Feng Lee March 13, 2024, 12:56 a.m. UTC | #2 
On 3/12/24 16:10, Andrii Nakryiko wrote:
> On Tue, Mar 12, 2024 at 11:32 AM Kui-Feng Lee <thinker.li@gmail.com> wrote:
>>
>> A new version of a type may have additional fields that do not exist in
>> older versions. Previously, libbpf would reject struct_ops maps with a new
>> version containing extra fields when running on a machine with an old
>> kernel. However, we have updated libbpf to ignore these fields if their
>> values are all zeros or null in order to provide backward compatibility.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>>   .../bpf/prog_tests/test_struct_ops_module.c   | 35 +++++++++++++++++++
>>   .../selftests/bpf/progs/struct_ops_module.c   | 13 +++++++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
>> index ee5372c7f2c7..e0d9ff75121b 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
>> @@ -93,9 +93,44 @@ static void test_struct_ops_load(void)
>>          struct_ops_module__destroy(skel);
>>   }
>>
>> +static void test_struct_ops_not_zeroed(void)
>> +{
>> +       struct struct_ops_module *skel;
>> +       int err;
>> +
>> +       skel = struct_ops_module__open();
>> +       if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
>> +               return;
>> +
>> +       bpf_program__set_autoload(skel->progs.test_3, false);
> 
> maybe mark test_3 program in progs/struct_ops_module.c as default
> not-autoload (SEC("?struct_ops/test_3")), so you don't have to set
> this to false everywhere?

Sure! I forgot we have this new feature.

> 
>> +
>> +       err = struct_ops_module__load(skel);
>> +       struct_ops_module__destroy(skel);
>> +
>> +       if (!ASSERT_OK(err, "struct_ops_module_load"))
>> +               return;
>> +
>> +       skel = struct_ops_module__open();
>> +       if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_not_zeroed"))
>> +               return;
>> +
>> +       bpf_program__set_autoload(skel->progs.test_3, false);
>> +
>> +       /* libbpf should reject the testmod_zeroed since the value of its
>> +        * "zeroed" is non-zero.
>> +        */
>> +       skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef;
>> +       err = struct_ops_module__load(skel);
>> +       ASSERT_ERR(err, "struct_ops_module_load_not_zeroed");
>> +
>> +       struct_ops_module__destroy(skel);
>> +}
>> +
>>   void serial_test_struct_ops_module(void)
>>   {
>>          if (test__start_subtest("test_struct_ops_load"))
>>                  test_struct_ops_load();
>> +       if (test__start_subtest("test_struct_ops_not_zeroed"))
>> +               test_struct_ops_not_zeroed();
>>   }
>>
>> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
>> index 026cabfa7f1f..d7d7606f639c 100644
>> --- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
>> +++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
>> @@ -54,3 +54,16 @@ struct bpf_testmod_ops___v2 testmod_2 = {
>>          .test_1 = (void *)test_1,
>>          .test_2 = (void *)test_2_v2,
>>   };
>> +
>> +struct bpf_testmod_ops___zeroed {
>> +       int (*test_1)(void);
>> +       void (*test_2)(int a, int b);
>> +       int (*test_maybe_null)(int dummy, struct task_struct *task);
>> +       int zeroed;
>> +};
>> +
>> +SEC(".struct_ops.link")
>> +struct bpf_testmod_ops___zeroed testmod_zeroed = {
>> +       .test_1 = (void *)test_1,
>> +       .test_2 = (void *)test_2_v2,
>> +};
> 
> We should also test the case where we have local ops definition with
> incompatible callback function signature (e.g., extra argument or
> something). Test then would update the program pointer (through
> skeleton's shadow type) to a compatible implementation.
> 
> Can you please add a test like that? Because the goal is to have a
> single struct_ops definition, if possible, and adjust it at runtime to
> make it compatible with the old kernel, let's have a small demo of
> this working.

Do you want to check signatures at libbpf? Or you just want a small
demo?

For extra arguments, IIRC, the verifier should reject the program if the 
program did access these arguments since it accesses memory beyond the 
last byte of the context. Doing extra checking at libbpf can provide 
better error messages if that is what you want.

If a program never accesses an extra argument, it should be allowed.(?)
WDYT?


> 
>> --
>> 2.34.1
>>
Andrii Nakryiko March 13, 2024, 3:52 p.m. UTC | #3 
On Tue, Mar 12, 2024 at 5:56 PM Kui-Feng Lee <sinquersw@gmail.com> wrote:
>
>
>
> On 3/12/24 16:10, Andrii Nakryiko wrote:
> > On Tue, Mar 12, 2024 at 11:32 AM Kui-Feng Lee <thinker.li@gmail.com> wrote:
> >>
> >> A new version of a type may have additional fields that do not exist in
> >> older versions. Previously, libbpf would reject struct_ops maps with a new
> >> version containing extra fields when running on a machine with an old
> >> kernel. However, we have updated libbpf to ignore these fields if their
> >> values are all zeros or null in order to provide backward compatibility.
> >>
> >> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> >> ---
> >>   .../bpf/prog_tests/test_struct_ops_module.c   | 35 +++++++++++++++++++
> >>   .../selftests/bpf/progs/struct_ops_module.c   | 13 +++++++
> >>   2 files changed, 48 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> >> index ee5372c7f2c7..e0d9ff75121b 100644
> >> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> >> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> >> @@ -93,9 +93,44 @@ static void test_struct_ops_load(void)
> >>          struct_ops_module__destroy(skel);
> >>   }
> >>
> >> +static void test_struct_ops_not_zeroed(void)
> >> +{
> >> +       struct struct_ops_module *skel;
> >> +       int err;
> >> +
> >> +       skel = struct_ops_module__open();
> >> +       if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
> >> +               return;
> >> +
> >> +       bpf_program__set_autoload(skel->progs.test_3, false);
> >
> > maybe mark test_3 program in progs/struct_ops_module.c as default
> > not-autoload (SEC("?struct_ops/test_3")), so you don't have to set
> > this to false everywhere?
>
> Sure! I forgot we have this new feature.
>
> >
> >> +
> >> +       err = struct_ops_module__load(skel);
> >> +       struct_ops_module__destroy(skel);
> >> +
> >> +       if (!ASSERT_OK(err, "struct_ops_module_load"))
> >> +               return;
> >> +
> >> +       skel = struct_ops_module__open();
> >> +       if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_not_zeroed"))
> >> +               return;
> >> +
> >> +       bpf_program__set_autoload(skel->progs.test_3, false);
> >> +
> >> +       /* libbpf should reject the testmod_zeroed since the value of its
> >> +        * "zeroed" is non-zero.
> >> +        */
> >> +       skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef;
> >> +       err = struct_ops_module__load(skel);
> >> +       ASSERT_ERR(err, "struct_ops_module_load_not_zeroed");
> >> +
> >> +       struct_ops_module__destroy(skel);
> >> +}
> >> +
> >>   void serial_test_struct_ops_module(void)
> >>   {
> >>          if (test__start_subtest("test_struct_ops_load"))
> >>                  test_struct_ops_load();
> >> +       if (test__start_subtest("test_struct_ops_not_zeroed"))
> >> +               test_struct_ops_not_zeroed();
> >>   }
> >>
> >> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
> >> index 026cabfa7f1f..d7d7606f639c 100644
> >> --- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
> >> +++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
> >> @@ -54,3 +54,16 @@ struct bpf_testmod_ops___v2 testmod_2 = {
> >>          .test_1 = (void *)test_1,
> >>          .test_2 = (void *)test_2_v2,
> >>   };
> >> +
> >> +struct bpf_testmod_ops___zeroed {
> >> +       int (*test_1)(void);
> >> +       void (*test_2)(int a, int b);
> >> +       int (*test_maybe_null)(int dummy, struct task_struct *task);
> >> +       int zeroed;
> >> +};
> >> +
> >> +SEC(".struct_ops.link")
> >> +struct bpf_testmod_ops___zeroed testmod_zeroed = {
> >> +       .test_1 = (void *)test_1,
> >> +       .test_2 = (void *)test_2_v2,
> >> +};
> >
> > We should also test the case where we have local ops definition with
> > incompatible callback function signature (e.g., extra argument or
> > something). Test then would update the program pointer (through
> > skeleton's shadow type) to a compatible implementation.
> >
> > Can you please add a test like that? Because the goal is to have a
> > single struct_ops definition, if possible, and adjust it at runtime to
> > make it compatible with the old kernel, let's have a small demo of
> > this working.
>
> Do you want to check signatures at libbpf? Or you just want a small
> demo?

Small demo/test was my goal here, to make sure this scenario works as expected.

>
> For extra arguments, IIRC, the verifier should reject the program if the
> program did access these arguments since it accesses memory beyond the
> last byte of the context. Doing extra checking at libbpf can provide
> better error messages if that is what you want.

I think libbpf could have checked local callback signature and kernel
callback signature and emit warning. If we don't enforce callback
signature match today and no one complained about that, I'd leave it
as is.

So let's start with a test for now and no more libbpf changes.


>
> If a program never accesses an extra argument, it should be allowed.(?)
> WDYT?
>
>
> >
> >> --
> >> 2.34.1
> >>
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
index ee5372c7f2c7..e0d9ff75121b 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
@@ -93,9 +93,44 @@  static void test_struct_ops_load(void)
 	struct_ops_module__destroy(skel);
 }
 
+static void test_struct_ops_not_zeroed(void)
+{
+	struct struct_ops_module *skel;
+	int err;
+
+	skel = struct_ops_module__open();
+	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.test_3, false);
+
+	err = struct_ops_module__load(skel);
+	struct_ops_module__destroy(skel);
+
+	if (!ASSERT_OK(err, "struct_ops_module_load"))
+		return;
+
+	skel = struct_ops_module__open();
+	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open_not_zeroed"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.test_3, false);
+
+	/* libbpf should reject the testmod_zeroed since the value of its
+	 * "zeroed" is non-zero.
+	 */
+	skel->struct_ops.testmod_zeroed->zeroed = 0xdeadbeef;
+	err = struct_ops_module__load(skel);
+	ASSERT_ERR(err, "struct_ops_module_load_not_zeroed");
+
+	struct_ops_module__destroy(skel);
+}
+
 void serial_test_struct_ops_module(void)
 {
 	if (test__start_subtest("test_struct_ops_load"))
 		test_struct_ops_load();
+	if (test__start_subtest("test_struct_ops_not_zeroed"))
+		test_struct_ops_not_zeroed();
 }
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
index 026cabfa7f1f..d7d7606f639c 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
@@ -54,3 +54,16 @@  struct bpf_testmod_ops___v2 testmod_2 = {
 	.test_1 = (void *)test_1,
 	.test_2 = (void *)test_2_v2,
 };
+
+struct bpf_testmod_ops___zeroed {
+	int (*test_1)(void);
+	void (*test_2)(int a, int b);
+	int (*test_maybe_null)(int dummy, struct task_struct *task);
+	int zeroed;
+};
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops___zeroed testmod_zeroed = {
+	.test_1 = (void *)test_1,
+	.test_2 = (void *)test_2_v2,
+};