Re: [PATCH net-next 02/10] cxgb4: flower: refuse a tunnel key wider than the VNI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] cxgb4: unvalidated tunnel key mask in
  cxgb4_validate_flow_match()
- [Low] cxgb4: missing Fixes trailer for tunnel VNI truncation fix
--

commit ffa1601309a5cbc17d72770bb794d0648b47f21f
Author: Alexandre Cassen <acassen@corp.free.fr>

> cxgb4: flower: refuse a tunnel key wider than the VNI
>
> cxgb4 holds a 24 bit VNI and truncates the key and the mask of enc_key_id.
> An exact key above bit 23 already selects the wrong VNI. Flower is about
> to accept partial masks. A mask that only covers bits above 23 would then
> clear the VNI mask and encap_vld. The rule would match any traffic its
> other keys allow. Refuse a key that no VNI can match.
>
> Found by an AI code review agent while auditing the users of enc_key_id.
> Build tested only.
>
> Signed-off-by: Alexandre Cassen <acassen@corp.free.fr>

[Severity: Low]
Since this also fixes the truncation of exact tunnel keys above bit 23,
should it include:

  Fixes: 98f3697f8d41 ("cxgb4: add tc flower match support for tunnel VNI")

> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> index 3307e50426819..d9a509be5c517 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c
> @@ -316,6 +316,17 @@ static int cxgb4_validate_flow_match(struct netlink_ext_ack *extack,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_KEYID)) {
> +		struct flow_match_enc_keyid match;
> +
> +		flow_rule_match_enc_keyid(rule, &match);
> +		if (be32_to_cpu(match.key->keyid) >> ENCAP_VNI_BITWIDTH) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Tunnel key does not fit the 24 bit VNI");
> +			return -EOPNOTSUPP;
> +		}
> +	}

[Severity: Medium]
Should this check match.mask->keyid instead of (or in addition to)
match.key->keyid?

When a rule matches on bits above bit 23 being zero (for example,
keyid = 0 and mask = 0xff000000), match.key->keyid is 0 and passes this
check.  cxgb4_process_flow_match() then truncates the 32-bit mask into
the 24-bit fs->mask.vni bitfield:

drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c:cxgb4_process_flow_match() {
    ...
		fs->val.vni = be32_to_cpu(match.key->keyid);
		fs->mask.vni = be32_to_cpu(match.mask->keyid);
		if (fs->mask.vni) {
			fs->val.encap_vld = 1;
			fs->mask.encap_vld = 1;
		}
    ...
}

Because fs->mask.vni becomes 0, encap_vld is left unset and the offloaded
filter matches any traffic allowed by the remaining keys.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109131039.959438-1-acassen@corp.free.fr?part=2