openvswitch: validate MPLS set/set_masked payload length

[PATCH net] openvswitch: validate MPLS set/set_masked payload length

Posted by Yang Yang 2 weeks, 4 days ago

validate_set() accepted OVS_KEY_ATTR_MPLS as variable-sized payload for
SET/SET_MASKED actions. In action handling, OVS expects fixed-size
MPLS key data (struct ovs_key_mpls).

Use the already normalized key_len (masked case included) and reject
non-matching MPLS action key sizes.

Reject invalid MPLS action payload lengths early.

Fixes: fbdcdd78da7c ("Change in Openvswitch to support MPLS label depth of 3 in ingress direction")
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Tested-by: Ao Zhou <n05ec@lzu.edu.cn>
Co-developed-by: Yuan Tan <tanyuan98@outlook.com>
Signed-off-by: Yuan Tan <tanyuan98@outlook.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Signed-off-by: Yang Yang <n05ec@lzu.edu.cn>
---

 net/openvswitch/flow_netlink.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 67fbf6e48a301..45444b81e9b97 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2953,6 +2953,8 @@ static int validate_set(const struct nlattr *a,
 	case OVS_KEY_ATTR_MPLS:
 		if (!eth_p_mpls(eth_type))
 			return -EINVAL;
+		if (key_len != sizeof(struct ovs_key_mpls))
+			return -EINVAL;
 		break;
 
 	case OVS_KEY_ATTR_SCTP:
-- 
2.43.0

Re: [PATCH net] openvswitch: validate MPLS set/set_masked payload length

Posted by Ilya Maximets 2 weeks, 4 days ago

On 3/19/26 9:02 AM, Yang Yang wrote:
> validate_set() accepted OVS_KEY_ATTR_MPLS as variable-sized payload for
> SET/SET_MASKED actions. In action handling, OVS expects fixed-size
> MPLS key data (struct ovs_key_mpls).
> 
> Use the already normalized key_len (masked case included) and reject
> non-matching MPLS action key sizes.
> 
> Reject invalid MPLS action payload lengths early.
> 
> Fixes: fbdcdd78da7c ("Change in Openvswitch to support MPLS label depth of 3 in ingress direction")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Tested-by: Ao Zhou <n05ec@lzu.edu.cn>
> Co-developed-by: Yuan Tan <tanyuan98@outlook.com>
> Signed-off-by: Yuan Tan <tanyuan98@outlook.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Signed-off-by: Yang Yang <n05ec@lzu.edu.cn>
> ---
> 
>  net/openvswitch/flow_netlink.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 67fbf6e48a301..45444b81e9b97 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -2953,6 +2953,8 @@ static int validate_set(const struct nlattr *a,
>  	case OVS_KEY_ATTR_MPLS:
>  		if (!eth_p_mpls(eth_type))
>  			return -EINVAL;
> +		if (key_len != sizeof(struct ovs_key_mpls))
> +			return -EINVAL;
>  		break;
>  
>  	case OVS_KEY_ATTR_SCTP:

Yeah, the check was not adjusted on the actions side when support for
multiple labels was added for the match.  Thanks for fixing this!
I ran OVS testsuite with this as well, and it works fine.

Reviewed-by: Ilya Maximets <i.maximets@ovn.org>