Re: [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/21/22 7:31 AM, Daniel Borkmann wrote:
On 3/20/22 4:20 AM, Yonghong Song wrote:
Compiled with clang by adding LLVM=1 both kernel and selftests/bpf
build, I hit the following compilation error:

In file included from /.../tools/testing/selftests/bpf/prog_tests/subskeleton.c:6:    ./test_subskeleton_lib.subskel.h:168:6: error: variable 'err' is used uninitialized whenever
       'if' condition is true [-Werror,-Wsometimes-uninitialized]
           if (!s->progs)
               ^~~~~~~~~
   ./test_subskeleton_lib.subskel.h:181:11: note: uninitialized use occurs here
           errno = -err;
                    ^~~
   ./test_subskeleton_lib.subskel.h:168:2: note: remove the 'if' if its condition is always false
           if (!s->progs)
           ^~~~~~~~~~~~~~

The compilation error is triggered by the following code
         ...
         int err;

         obj = (struct test_subskeleton_lib *)calloc(1, sizeof(*obj));
         if (!obj) {
                 errno = ENOMEM;
                 goto err;
         }
         ...

   err:
         test_subskeleton_lib__destroy(obj);
         errno = -err;
         ...
in test_subskeleton_lib__open(). The 'err' is not initialized, yet it
is used in 'errno = -err' later.

The fix is to remove 'errno = -err' since errno has been set properly
in all incoming branches.

If we remove this one here in which locations is it missing then? Do these then
need an extra errno = -err statement before they goto err?

Everything should be covered. The following are all 'goto err' returns:

        obj = (struct test_subskeleton_lib *)calloc(1, sizeof(*obj));
        if (!obj) {
                errno = ENOMEM;
                goto err;
        }
        s = (struct bpf_object_subskeleton *)calloc(1, sizeof(*s));
        if (!s) {
                errno = ENOMEM;
                goto err;
        }
	...
        s->vars = (struct bpf_var_skeleton *)calloc(10, sizeof(*s->vars));
        if (!s->vars) {
                errno = ENOMEM;
                goto err;
        }
	...

==> for all maps

        /* maps */
        s->map_cnt = 7;
        s->map_skel_sz = sizeof(*s->maps);
s->maps = (struct bpf_map_skeleton *)calloc(s->map_cnt, s->map_skel_sz);
        if (!s->maps)
                goto err;
==> calloc should set error number properly if failed.
	...

==> for all progs
	/* programs */
        s->prog_cnt = 1;
        s->prog_skel_sz = sizeof(*s->progs);
s->progs = (struct bpf_prog_skeleton *)calloc(s->prog_cnt, s->prog_skel_sz);
        if (!s->progs)
                goto err;
==> calloc should set error number properly if failed.
	
        err = bpf_object__open_subskeleton(s);
        if (err)
                goto err;

	return obj;

==> bpf_object__open_subskeleton() in libbpf.c does set errno probably if 'err' is not 0.



Cc: Delyan Kratunov <delyank@xxxxxx>
Fixes: 00389c58ffe9 ("00389c58ffe993782a8ba4bb5a34a102b1f6fe24")
Signed-off-by: Yonghong Song <yhs@xxxxxx>
---
  tools/bpf/bpftool/gen.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 96bd2b33ccf6..7ba7ff55d2ea 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1538,7 +1538,6 @@ static int do_subskeleton(int argc, char **argv)
              return obj;                        \n\
          err:                                \n\
              %1$s__destroy(obj);                    \n\
-            errno = -err;                        \n\
              return NULL;                        \n\
          }                                \n\
                                          \n\





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux