| Message ID | 1629988201-41820-1-git-send-email-wangyunjian@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] odp-util: Fix a null pointer dereference | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/apply-robot | fail | apply and check: fail |
w00273186 <wangyunjian@huawei.com> writes: > From: Yunjian Wang <wangyunjian@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@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') > const struct nlattr *ma = nl_attr_find__(mask, mask_len, > OVS_KEY_ATTR_ETHERTYPE); > if (ma) {
> -----Original Message----- > From: Aaron Conole [mailto:aconole@redhat.com] > Sent: Wednesday, September 1, 2021 3:21 AM > To: wangyunjian <wangyunjian@huawei.com> > Cc: dev@openvswitch.org; i.maximets@ovn.org; dingxiaoxiong > <dingxiaoxiong@huawei.com> > Subject: Re: [ovs-dev] [PATCH] odp-util: Fix a null pointer dereference > > w00273186 <wangyunjian@huawei.com> writes: > > > From: Yunjian Wang <wangyunjian@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@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) {
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) { const struct nlattr *ma = nl_attr_find__(mask, mask_len, OVS_KEY_ATTR_ETHERTYPE); if (ma) {