[ovs-dev] [PATCH] odp-util: Fix a null pointer dereference

[wangyunjian]

> -----Original Message-----
> From: Aaron Conole [mailto:aconole at redhat.com]
> Sent: Wednesday, September 1, 2021 3:21 AM
> To: wangyunjian <wangyunjian at huawei.com>
> Cc: dev at openvswitch.org; i.maximets at ovn.org; dingxiaoxiong
> <dingxiaoxiong at huawei.com>
> Subject: Re: [ovs-dev] [PATCH] odp-util: Fix a null pointer dereference
> 
> w00273186 <wangyunjian at huawei.com> writes:
> 
> > From: Yunjian Wang <wangyunjian at huawei.com>
> >
> > This patch fixes (dereference after null check) coverity issue.
> > For this reason, we should add null check of 'mask' before calling
> > nl_attr_find__() because the 'mask' maybe null.
> >
> > Addresses-Coverity: ("Dereference after null check")
> > Fixes: e6cc0babc25d ("ovs-dpctl: Add mega flow support")
> >
> > Signed-off-by: Yunjian Wang <wangyunjian at huawei.com>
> > ---
> >  lib/odp-util.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/odp-util.c b/lib/odp-util.c index
> > 7729a9060..c0743800a 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -4618,7 +4618,7 @@ odp_flow_format(const struct nlattr *key, size_t
> key_len,
> >              }
> >              ds_put_char(ds, ')');
> >          }
> > -        if (!has_ethtype_key) {
> > +        if (!has_ethtype_key && mask && mask_len) {
> 
> Do we need to check mask_len here?  I guess it should be getting checked
> during nl_attr_is_valid, right?  Even still, we should probably be checking
> more than whether it is non-zero (something like '>= sizeof *ma')

Yes, I agree. It will be checked during nl_attr_is_valid(and >= sizeof *nla).
I will fix it in next version.

Thanks

> 
> >              const struct nlattr *ma = nl_attr_find__(mask, mask_len,
> >
> OVS_KEY_ATTR_ETHERTYPE);
> >              if (ma) {