Hi Aaron, I did not look at your comment before. You are right that, mask_len parameter is validated via nl_attr_is_valid(). It is ok not to check for mask_len.
Just that in the function odp_flow_format(), mask_len is used in 3 places. Two of the places have the check for it and this is the third one that I suggested to have the check. I am ok with not checking. Acked-by: Vasu Dasari <[email protected]> Best, -Vasu *Vasu Dasari* On Wed, Sep 1, 2021 at 9:09 AM Aaron Conole <[email protected]> wrote: > > Hi Vasu, > > Vasu Dasari <[email protected]> writes: > > > Good catch!!! > > Can you also check for mask_len before calling nl_attr_find__() as is > done > > in other places in the same function? > > I asked to remove the mask_len check. nl_attr_find__() already does > such check. I scanned the code to find such examples, but only one > exists (which is in odp-util.c). Other places either don't validate at > all, or were pulled during an nl_attr_get_size() call. > > I don't see why such check is needed. Maybe I missed something? > > > *Vasu Dasari* > > > > > > On Wed, Sep 1, 2021 at 2:29 AM Yunjian Wang <[email protected]> > wrote: > > > >> 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 <[email protected]> > >> --- > >> v2: > >> * update code styles suggested by Aaron Conole > >> --- > >> 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..bf427f027 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) { > >> const struct nlattr *ma = nl_attr_find__(mask, mask_len, > >> > >> OVS_KEY_ATTR_ETHERTYPE); > >> if (ma) { > >> -- > >> 2.18.1 > >> > >> _______________________________________________ > >> dev mailing list > >> [email protected] > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
