| Message ID | 20250220-bpf-uninit-v1-1-af07a5a57e5b@ethancedwards.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btf: move kern_type_id to goto cand_cache_unlock | expand |
On 2025/2/20 13:50, Ethan Carter Edwards wrote: > In most code paths variable move_kern_type_id remains uninitialized upon > return. By moving it to the goto, it is initialized in these code paths. > As well as others. Caught by Coverity. > > Closes: https://scan5.scan.coverity.com/#/project-view/63874/10063?selectedIssue=1595567 > Fixes: e2b3c4ff5d183d ("bpf: add __arg_trusted global func arg tag") > Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com> > --- > kernel/bpf/btf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index 9de6acddd479b4f5e32a5e6ba43cf369de4cee29..8c82ced7da299ad1ad769024fe097898c269013b 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -7496,9 +7496,9 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx, > err = -EOPNOTSUPP; > goto cand_cache_unlock; > } > - kern_type_id = cc->cands[0].id; > > cand_cache_unlock: > + kern_type_id = cc->cands[0].id; Hi, for goto's branches, it will always `return err`, no need to make this move. > mutex_unlock(&cand_cache_mutex); > if (err) > return err; > > --- > base-commit: 87a132e73910e8689902aed7f2fc229d6908383b > change-id: 20250220-bpf-uninit-3323a4426da9 > > Best regards,
On 25/02/20 08:24PM, Pu Lehui wrote: > On 2025/2/20 13:50, Ethan Carter Edwards wrote: > > In most code paths variable move_kern_type_id remains uninitialized upon > > return. By moving it to the goto, it is initialized in these code paths. > > As well as others. Caught by Coverity. > > > > Closes: https://scan5.scan.coverity.com/#/project-view/63874/10063?selectedIssue=1595567 > > Fixes: e2b3c4ff5d183d ("bpf: add __arg_trusted global func arg tag") > > Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com> > > --- > > kernel/bpf/btf.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > > index 9de6acddd479b4f5e32a5e6ba43cf369de4cee29..8c82ced7da299ad1ad769024fe097898c269013b 100644 > > --- a/kernel/bpf/btf.c > > +++ b/kernel/bpf/btf.c > > @@ -7496,9 +7496,9 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx, > > err = -EOPNOTSUPP; > > goto cand_cache_unlock; > > } > > - kern_type_id = cc->cands[0].id; > > cand_cache_unlock: > > + kern_type_id = cc->cands[0].id; > > Hi, for goto's branches, it will always `return err`, no need to make this > move. You are right. My apologies. I should probably do less coding at 2AM. Thanks, Ethan
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 9de6acddd479b4f5e32a5e6ba43cf369de4cee29..8c82ced7da299ad1ad769024fe097898c269013b 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -7496,9 +7496,9 @@ static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx, err = -EOPNOTSUPP; goto cand_cache_unlock; } - kern_type_id = cc->cands[0].id; cand_cache_unlock: + kern_type_id = cc->cands[0].id; mutex_unlock(&cand_cache_mutex); if (err) return err;
In most code paths variable move_kern_type_id remains uninitialized upon return. By moving it to the goto, it is initialized in these code paths. As well as others. Caught by Coverity. Closes: https://scan5.scan.coverity.com/#/project-view/63874/10063?selectedIssue=1595567 Fixes: e2b3c4ff5d183d ("bpf: add __arg_trusted global func arg tag") Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com> --- kernel/bpf/btf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 87a132e73910e8689902aed7f2fc229d6908383b change-id: 20250220-bpf-uninit-3323a4426da9 Best regards,