* [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation
@ 2022-03-20 3:20 Yonghong Song
2022-03-21 14:31 ` Daniel Borkmann
2022-03-21 21:50 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 4+ messages in thread
From: Yonghong Song @ 2022-03-20 3:20 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Delyan Kratunov
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.
Cc: Delyan Kratunov <delyank@fb.com>
Fixes: 00389c58ffe9 ("00389c58ffe993782a8ba4bb5a34a102b1f6fe24")
Signed-off-by: Yonghong Song <yhs@fb.com>
---
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\
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation
2022-03-20 3:20 [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation Yonghong Song
@ 2022-03-21 14:31 ` Daniel Borkmann
2022-03-21 15:39 ` Yonghong Song
2022-03-21 21:50 ` patchwork-bot+netdevbpf
1 sibling, 1 reply; 4+ messages in thread
From: Daniel Borkmann @ 2022-03-21 14:31 UTC (permalink / raw)
To: Yonghong Song, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, kernel-team, Delyan Kratunov
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?
> Cc: Delyan Kratunov <delyank@fb.com>
> Fixes: 00389c58ffe9 ("00389c58ffe993782a8ba4bb5a34a102b1f6fe24")
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> 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\
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation
2022-03-21 14:31 ` Daniel Borkmann
@ 2022-03-21 15:39 ` Yonghong Song
0 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2022-03-21 15:39 UTC (permalink / raw)
To: Daniel Borkmann, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, kernel-team, Delyan Kratunov
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@fb.com>
>> Fixes: 00389c58ffe9 ("00389c58ffe993782a8ba4bb5a34a102b1f6fe24")
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> 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\
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation
2022-03-20 3:20 [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation Yonghong Song
2022-03-21 14:31 ` Daniel Borkmann
@ 2022-03-21 21:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-03-21 21:50 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, ast, andrii, daniel, kernel-team, delyank
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Sat, 19 Mar 2022 20:20:09 -0700 you 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)
> ^~~~~~~~~~~~~~
>
> [...]
Here is the summary with links:
- [bpf-next] bpftool: fix a bug in subskeleton code generation
https://git.kernel.org/bpf/bpf-next/c/f97b8b9bd630
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-21 22:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-20 3:20 [PATCH bpf-next] bpftool: fix a bug in subskeleton code generation Yonghong Song
2022-03-21 14:31 ` Daniel Borkmann
2022-03-21 15:39 ` Yonghong Song
2022-03-21 21:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox