lkml.org 
[lkml]   [2020]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.9 05/46] net: openvswitch: fix TTL decrement action netlink message format
    Date
    From: Eelco Chaudron <echaudro@redhat.com>

    [ Upstream commit 69929d4c49e182f8526d42c43b37b460d562d3a0 ]

    Currently, the openvswitch module is not accepting the correctly formated
    netlink message for the TTL decrement action. For both setting and getting
    the dec_ttl action, the actions should be nested in the
    OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h uapi.

    When the original patch was sent, it was tested with a private OVS userspace
    implementation. This implementation was unfortunately not upstreamed and
    reviewed, hence an erroneous version of this patch was sent out.

    Leaving the patch as-is would cause problems as the kernel module could
    interpret additional attributes as actions and vice-versa, due to the
    actions not being encapsulated/nested within the actual attribute, but
    being concatinated after it.

    Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
    Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
    Link: https://lore.kernel.org/r/160622121495.27296.888010441924340582.stgit@wsfd-netdev64.ntdv.lab.eng.bos.redhat.com
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    include/uapi/linux/openvswitch.h | 2 +
    net/openvswitch/actions.c | 7 +--
    net/openvswitch/flow_netlink.c | 74 ++++++++++++++++++++++++++++-----------
    3 files changed, 60 insertions(+), 23 deletions(-)

    --- a/include/uapi/linux/openvswitch.h
    +++ b/include/uapi/linux/openvswitch.h
    @@ -1058,4 +1058,6 @@ enum ovs_dec_ttl_attr {
    __OVS_DEC_TTL_ATTR_MAX
    };

    +#define OVS_DEC_TTL_ATTR_MAX (__OVS_DEC_TTL_ATTR_MAX - 1)
    +
    #endif /* _LINUX_OPENVSWITCH_H */
    --- a/net/openvswitch/actions.c
    +++ b/net/openvswitch/actions.c
    @@ -970,14 +970,13 @@ static int dec_ttl_exception_handler(str
    {
    /* The first action is always 'OVS_DEC_TTL_ATTR_ARG'. */
    struct nlattr *dec_ttl_arg = nla_data(attr);
    - int rem = nla_len(attr);

    if (nla_len(dec_ttl_arg)) {
    - struct nlattr *actions = nla_next(dec_ttl_arg, &rem);
    + struct nlattr *actions = nla_data(dec_ttl_arg);

    if (actions)
    - return clone_execute(dp, skb, key, 0, actions, rem,
    - last, false);
    + return clone_execute(dp, skb, key, 0, nla_data(actions),
    + nla_len(actions), last, false);
    }
    consume_skb(skb);
    return 0;
    --- a/net/openvswitch/flow_netlink.c
    +++ b/net/openvswitch/flow_netlink.c
    @@ -2503,28 +2503,42 @@ static int validate_and_copy_dec_ttl(str
    __be16 eth_type, __be16 vlan_tci,
    u32 mpls_label_count, bool log)
    {
    - int start, err;
    - u32 nested = true;
    + const struct nlattr *attrs[OVS_DEC_TTL_ATTR_MAX + 1];
    + int start, action_start, err, rem;
    + const struct nlattr *a, *actions;
    +
    + memset(attrs, 0, sizeof(attrs));
    + nla_for_each_nested(a, attr, rem) {
    + int type = nla_type(a);
    +
    + /* Ignore unknown attributes to be future proof. */
    + if (type > OVS_DEC_TTL_ATTR_MAX)
    + continue;

    - if (!nla_len(attr))
    - return ovs_nla_add_action(sfa, OVS_ACTION_ATTR_DEC_TTL,
    - NULL, 0, log);
    + if (!type || attrs[type])
    + return -EINVAL;
    +
    + attrs[type] = a;
    + }
    +
    + actions = attrs[OVS_DEC_TTL_ATTR_ACTION];
    + if (rem || !actions || (nla_len(actions) && nla_len(actions) < NLA_HDRLEN))
    + return -EINVAL;

    start = add_nested_action_start(sfa, OVS_ACTION_ATTR_DEC_TTL, log);
    if (start < 0)
    return start;

    - err = ovs_nla_add_action(sfa, OVS_DEC_TTL_ATTR_ACTION, &nested,
    - sizeof(nested), log);
    -
    - if (err)
    - return err;
    + action_start = add_nested_action_start(sfa, OVS_DEC_TTL_ATTR_ACTION, log);
    + if (action_start < 0)
    + return start;

    - err = __ovs_nla_copy_actions(net, attr, key, sfa, eth_type,
    + err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
    vlan_tci, mpls_label_count, log);
    if (err)
    return err;

    + add_nested_action_end(*sfa, action_start);
    add_nested_action_end(*sfa, start);
    return 0;
    }
    @@ -3487,20 +3501,42 @@ out:
    static int dec_ttl_action_to_attr(const struct nlattr *attr,
    struct sk_buff *skb)
    {
    - int err = 0, rem = nla_len(attr);
    - struct nlattr *start;
    + struct nlattr *start, *action_start;
    + const struct nlattr *a;
    + int err = 0, rem;

    start = nla_nest_start_noflag(skb, OVS_ACTION_ATTR_DEC_TTL);
    -
    if (!start)
    return -EMSGSIZE;

    - err = ovs_nla_put_actions(nla_data(attr), rem, skb);
    - if (err)
    - nla_nest_cancel(skb, start);
    - else
    - nla_nest_end(skb, start);
    + nla_for_each_attr(a, nla_data(attr), nla_len(attr), rem) {
    + switch (nla_type(a)) {
    + case OVS_DEC_TTL_ATTR_ACTION:
    +
    + action_start = nla_nest_start_noflag(skb, OVS_DEC_TTL_ATTR_ACTION);
    + if (!action_start) {
    + err = -EMSGSIZE;
    + goto out;
    + }

    + err = ovs_nla_put_actions(nla_data(a), nla_len(a), skb);
    + if (err)
    + goto out;
    +
    + nla_nest_end(skb, action_start);
    + break;
    +
    + default:
    + /* Ignore all other option to be future compatible */
    + break;
    + }
    + }
    +
    + nla_nest_end(skb, start);
    + return 0;
    +
    +out:
    + nla_nest_cancel(skb, start);
    return err;
    }


    \
     
     \ /
      Last update: 2020-12-06 12:45    [W:2.581 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site