[PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[Joe Stringer]

Set actions consist of a regular OVS_KEY_ATTR_* attribute nested inside
of a OVS_ACTION_ATTR_SET action attribute. When converting masked actions
back to regular set actions, the inner attribute length was not changed,
ie, double the length being serialized. This patch fixes the bug.

Fixes: 83d2b9b ("net: openvswitch: Support masked set actions.")
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
This fix needs to be applied to net as well; it applies cleanly.
---
 net/openvswitch/flow_netlink.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 216f20b..22b18c1 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2253,14 +2253,20 @@ static int masked_set_action_to_set_action_attr(const struct nlattr *a,
 						struct sk_buff *skb)
 {
 	const struct nlattr *ovs_key = nla_data(a);
+	struct nlattr *nla;
 	size_t key_len = nla_len(ovs_key) / 2;
 
 	/* Revert the conversion we did from a non-masked set action to
 	 * masked set action.
 	 */
-	if (nla_put(skb, OVS_ACTION_ATTR_SET, nla_len(a) - key_len, ovs_key))
+	nla = nla_nest_start(skb, OVS_ACTION_ATTR_SET);
+	if (!nla)
 		return -EMSGSIZE;
 
+	if (nla_put(skb, nla_type(ovs_key), key_len, nla_data(ovs_key)))
+		return -EMSGSIZE;
+
+	nla_nest_end(skb, nla);
 	return 0;
 }
 
-- 
1.7.10.4

Re: [PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[Jarno Rajahalme]

Thanks for spotting and fixing this,

Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

> On Mar 2, 2015, at 6:49 PM, Joe Stringer <joestringer@nicira.com> wrote:
> 
> Set actions consist of a regular OVS_KEY_ATTR_* attribute nested inside
> of a OVS_ACTION_ATTR_SET action attribute. When converting masked actions
> back to regular set actions, the inner attribute length was not changed,
> ie, double the length being serialized. This patch fixes the bug.
> 
> Fixes: 83d2b9b ("net: openvswitch: Support masked set actions.")
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> This fix needs to be applied to net as well; it applies cleanly.
> ---
> net/openvswitch/flow_netlink.c |    8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 216f20b..22b18c1 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -2253,14 +2253,20 @@ static int masked_set_action_to_set_action_attr(const struct nlattr *a,
> 						struct sk_buff *skb)
> {
> 	const struct nlattr *ovs_key = nla_data(a);
> +	struct nlattr *nla;
> 	size_t key_len = nla_len(ovs_key) / 2;
> 
> 	/* Revert the conversion we did from a non-masked set action to
> 	 * masked set action.
> 	 */
> -	if (nla_put(skb, OVS_ACTION_ATTR_SET, nla_len(a) - key_len, ovs_key))
> +	nla = nla_nest_start(skb, OVS_ACTION_ATTR_SET);
> +	if (!nla)
> 		return -EMSGSIZE;
> 
> +	if (nla_put(skb, nla_type(ovs_key), key_len, nla_data(ovs_key)))
> +		return -EMSGSIZE;
> +
> +	nla_nest_end(skb, nla);
> 	return 0;
> }
> 
> -- 
> 1.7.10.4
> 

Re: [PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[David Miller]

From: Joe Stringer <joestringer@nicira.com>
Date: Mon,  2 Mar 2015 18:49:56 -0800

> Set actions consist of a regular OVS_KEY_ATTR_* attribute nested inside
> of a OVS_ACTION_ATTR_SET action attribute. When converting masked actions
> back to regular set actions, the inner attribute length was not changed,
> ie, double the length being serialized. This patch fixes the bug.
> 
> Fixes: 83d2b9b ("net: openvswitch: Support masked set actions.")
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> This fix needs to be applied to net as well; it applies cleanly.

This is backwards, don't do this.  Just say that 'net' needs the fix
and submit it targetting that tree only.  Things in 'net' propagate
into 'net-next' automatically as I do merges over time, but the
opposite is not true.

Re: [PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[Joe Stringer]

On 3 March 2015 at 11:39, David Miller <davem@davemloft.net> wrote:
> From: Joe Stringer <joestringer@nicira.com>
> Date: Mon,  2 Mar 2015 18:49:56 -0800
>
>> Set actions consist of a regular OVS_KEY_ATTR_* attribute nested inside
>> of a OVS_ACTION_ATTR_SET action attribute. When converting masked actions
>> back to regular set actions, the inner attribute length was not changed,
>> ie, double the length being serialized. This patch fixes the bug.
>>
>> Fixes: 83d2b9b ("net: openvswitch: Support masked set actions.")
>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>> ---
>> This fix needs to be applied to net as well; it applies cleanly.
>
> This is backwards, don't do this.  Just say that 'net' needs the fix
> and submit it targetting that tree only.  Things in 'net' propagate
> into 'net-next' automatically as I do merges over time, but the
> opposite is not true.

OK, I'll resend.

Re: [PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[Joe Stringer]

On 3 March 2015 at 12:38, Joe Stringer <joestringer@nicira.com> wrote:
> On 3 March 2015 at 11:39, David Miller <davem@davemloft.net> wrote:
>> From: Joe Stringer <joestringer@nicira.com>
>> Date: Mon,  2 Mar 2015 18:49:56 -0800
>>
>>> Set actions consist of a regular OVS_KEY_ATTR_* attribute nested inside
>>> of a OVS_ACTION_ATTR_SET action attribute. When converting masked actions
>>> back to regular set actions, the inner attribute length was not changed,
>>> ie, double the length being serialized. This patch fixes the bug.
>>>
>>> Fixes: 83d2b9b ("net: openvswitch: Support masked set actions.")
>>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>>> ---
>>> This fix needs to be applied to net as well; it applies cleanly.
>>
>> This is backwards, don't do this.  Just say that 'net' needs the fix
>> and submit it targetting that tree only.  Things in 'net' propagate
>> into 'net-next' automatically as I do merges over time, but the
>> opposite is not true.
>
> OK, I'll resend.

Nevermind, I see that you picked it up anyway. Thanks, I'll send fixes
to the appropriate branch in future.

Re: [PATCH net-next] openvswitch: Fix serialization of non-masked set actions.

[David Miller]

From: Joe Stringer <joestringer@nicira.com>
Date: Tue, 3 Mar 2015 12:38:54 -0800

> On 3 March 2015 at 11:39, David Miller <davem@davemloft.net> wrote:
>> From: Joe Stringer <joestringer@nicira.com>
>> Date: Mon,  2 Mar 2015 18:49:56 -0800
>>
>>> Set actions consist of a regular OVS_KEY_ATTR_* attribute nested inside
>>> of a OVS_ACTION_ATTR_SET action attribute. When converting masked actions
>>> back to regular set actions, the inner attribute length was not changed,
>>> ie, double the length being serialized. This patch fixes the bug.
>>>
>>> Fixes: 83d2b9b ("net: openvswitch: Support masked set actions.")
>>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>>> ---
>>> This fix needs to be applied to net as well; it applies cleanly.
>>
>> This is backwards, don't do this.  Just say that 'net' needs the fix
>> and submit it targetting that tree only.  Things in 'net' propagate
>> into 'net-next' automatically as I do merges over time, but the
>> opposite is not true.
> 
> OK, I'll resend.

If you actually checked the 'net' tree, I applied your patch and even sent
it off to Linus via a pull request 15 minutes ago.