[dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod

Ananyev, Konstantin konstantin.ananyev at intel.com
Wed Nov 7 21:07:54 CET 2018



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Wednesday, November 7, 2018 7:52 PM
> To: Ananyev, Konstantin <konstantin.ananyev at intel.com>
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod
> 
> On Wed, 7 Nov 2018 12:54:54 +0000
> "Ananyev, Konstantin" <konstantin.ananyev at intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen Hemminger
> > > Sent: Tuesday, November 6, 2018 9:49 PM
> > > To: dev at dpdk.org
> > > Cc: Stephen Hemminger <stephen at networkplumber.org>
> > > Subject: [dpdk-dev] [RFC 3/5] bpf: fix validation of eal_divmod
> > >
> > > Coverity spotted self assignment in BPF eval_divmod.
> >
> > Yep, there is one.
> > As I remember I have to add it because one of old versions
> > of compiler (clang???) complained about 'variable being used uninitialized'.
> >
> > > This looks like a bug where the incoming source register
> > > should have been used instead.
> >
> > Nope, that's a wrong guess.
> > We shouldn't do it here.
> > Konstantin
> >
> > >
> > > Coverity issue: 302850
> > > Fixes: 8021917293d0 ("bpf: add extra validation for input BPF program")
> > > Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
> > > ---
> > >  lib/librte_bpf/bpf_validate.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_bpf/bpf_validate.c b/lib/librte_bpf/bpf_validate.c
> > > index 83983efc4e5c..b768f72c4c02 100644
> > > --- a/lib/librte_bpf/bpf_validate.c
> > > +++ b/lib/librte_bpf/bpf_validate.c
> > > @@ -512,7 +512,7 @@ eval_divmod(uint32_t op, struct bpf_reg_val *rd, struct bpf_reg_val *rs,
> > >  		if (op == BPF_MOD)
> > >  			rd->u.max = RTE_MIN(rd->u.max, rs->u.max - 1);
> > >  		else
> > > -			rd->u.max = rd->u.max;
> > > +			rd->u.max = rs->u.max;
> > >  		rd->u.min = 0;
> > >  	}
> > >
> > > --
> > > 2.17.1
> >
> 
> Well it was being used unintialized, 

I don't think so, but if you can point to me where
exactly it is used uninitialized, we can discuss it further.

> your trick of self assignment fooled clang

It was one particular and pretty old version of clang
(if my memory serves me right).
With latest versions (let say 6.0) it doesn't complain,
if I remove that self-assignment.
gcc also doesn't see any problem here.
That makes me think it was a false-positive with old
version of the compiler.
Konstantin 

> but did not fool Coverity.  What does the other BPF validator do?


More information about the dev mailing list