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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, November 7, 2018 7:52 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@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@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > > Sent: Tuesday, November 6, 2018 9:49 PM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger <stephen@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@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?