[ovs-dev] [PATCH 1/2] datapath: Fix Flow dump operation.

[Jesse Gross]

On Sat, Jan 19, 2013 at 8:15 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> Following patch adds null check while inserting new netlink attribute.
> This was introduced by commit 9b405f1aa8d175d (datapath: More
> flexible kernel/userspace tunneling attribute.)
>
> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
> Bug #14767
> ---
>  datapath/datapath.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index b8e9d18..ed3dd09 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -1109,10 +1109,15 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
>          * properly sized for single flows.
>          */
>         start = nla_nest_start(skb, OVS_FLOW_ATTR_ACTIONS);
> -       err = actions_to_attr(sf_acts->actions, sf_acts->actions_len, skb);
> -       if (err < 0 && skb_orig_len)
> +       if (start) {
> +               err = actions_to_attr(sf_acts->actions, sf_acts->actions_len, skb);
> +               if (err < 0 && skb_orig_len)
> +                       goto error;
> +               nla_nest_end(skb, start);
> +       } else if (skb_orig_len) {
> +               err = -ENOMEM;
>                 goto error;

I think the correct error code to return here is -EMSGSIZE (or we
could just jump to nla_put_failure).  Otherwise, I agree with Ben's
comments.