APPLIED[backlog]/cmnt: [SRU][X][PATCH 0/1] Fix for bpf static analysis warning (LP#1745364)

Kleber Souza kleber.souza at canonical.com
Wed Feb 28 14:17:52 UTC 2018


On 01/25/18 13:58, Daniel Axtens wrote:
> SRU Justification
> =================
> 
> Coverity reports:
> 
> *** CID 1464330: Uninitialized variables (MISSING_RETURN)
> /arch/x86/net/bpf_jit_comp.c: 1088 in bpf_int_jit_compile()
> 1082 int i;
> 1083 1084 if (!bpf_jit_enable)
> 1085 return prog;
> 1086 1087 if (!prog || !prog->len)
>>>> CID 1464330: Uninitialized variables (MISSING_RETURN)
>>>> Arriving at the end of a function without returning a value.
> 1088 return;
> 1089 1090 addrs = kmalloc(prog->len * sizeof(*addrs), GFP_KERNEL);
> 1091 if (!addrs)
> 1092 return prog;
> 1093
> 
> This is a result of 3098d8eae421 ("bpf: prepare
> bpf_int_jit_compile/bpf_prog_select_runtime apis"), which is a
> cherry-pick of d1c55ab5e41f upstream. In that patch, the return type
> of bpf_int_jit_compile was changed from void to struct bpf_prog*. That
> patch changed some of the return statements.
> 
> It did not, however, change the return statement of the (!prog ||
> !prog->len) check, as in upstream the (!prog || !prog->len) check was
> dropped in 93a73d442d37 ("bpf, x86/arm64: remove useless checks on
> prog"):
> 
> """
> There is never such a situation, where bpf_int_jit_compile() is
> called with either prog as NULL or len as 0, so the tests are
> unnecessary and confusing as people would just copy them.
> """
> 
> However, we haven't picked up 93a73d442d37, so when we cherry-picked
> d1c55ab5e41f, that branch remained unmodified, hence the static
> analysis warning.
> 
> Impact
> ======
> 
> If the branch is not dead and someone can hit it, an undefined value
> can be returned, which could cause issues.
> 
> Fix
> ===
> 
> For consistency and in case the branch is not actually dead on Xenial,
> we should do a fixup to 'return prog;'
> 
> Regression Potential
> ====================
> 
> Limited to the BPF jit which is off by default.
> Limited to a branch that should be dead code anyway.
> Limited to an error handling path.
> 
> Daniel Axtens (1):
>   UBUNTU: SAUCE: (no-up) arch/x86/bpf: Fix missed return statement
> 
>  arch/x86/net/bpf_jit_comp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied to xenial/master-next-backlog, fixing context (fuzzing).

Thanks,
Kleber




More information about the kernel-team mailing list