Hi,
On Mon, 22 Aug 2016 17:38:34 +0300 Amir Vadai <a...@vadai.me> wrote:
> +static struct metadata_dst *iptunnel_alloc(struct tcf_iptunnel *t,
> + __be32 saddr, __be32 daddr,
> + __be64 key_id)
> +{
> + struct ip_tunnel_info *tun_info;
> + struct metadata_dst *metadata;
> +
> + metadata = metadata_dst_alloc(0, GFP_KERNEL);
> + if (!metadata)
> + return ERR_PTR(-ENOMEM);
> +
> + tun_info = &metadata->u.tun_info;
> + tun_info->mode = IP_TUNNEL_INFO_TX;
>
> + ip_tunnel_key_init(&tun_info->key, saddr, daddr, 0, 0, 0, 0, 0,
> + key_id, 0);
Seems key.tun_flags should be armed with TUNNEL_KEY.
This will make things work with GRE as well.
Pass it in the 'tun_flags' parameter.
> +
> + return metadata;
> +}
> +
> +static int tcf_iptunnel_init(struct net *net, struct nlattr *nla,
> + struct nlattr *est, struct tc_action **a,
> + int ovr, int bind)
> +{
> + struct tc_action_net *tn = net_generic(net, iptunnel_net_id);
> + struct nlattr *tb[TCA_IPTUNNEL_MAX + 1];
> + struct metadata_dst *metadata;
> + struct tc_iptunnel *parm;
> + struct tcf_iptunnel *t;
> + __be32 saddr = 0;
> + __be32 daddr = 0;
> + __be64 key_id = 0;
> + int encapdecap;
> + bool exists = false;
> + int ret = -EINVAL;
> + int err;
> +
> + if (!nla)
> + return -EINVAL;
> +
> + err = nla_parse_nested(tb, TCA_IPTUNNEL_MAX, nla, iptunnel_policy);
> + if (err < 0)
> + return err;
> +
> + if (!tb[TCA_IPTUNNEL_PARMS])
> + return -EINVAL;
> + parm = nla_data(tb[TCA_IPTUNNEL_PARMS]);
> + exists = tcf_hash_check(tn, parm->index, a, bind);
> + if (exists && bind)
> + return 0;
> +
> + encapdecap = parm->t_action;
> +
> + switch (encapdecap) {
> + case TCA_IPTUNNEL_ACT_DECAP:
> + break;
> + case TCA_IPTUNNEL_ACT_ENCAP:
> + if (tb[TCA_IPTUNNEL_ENC_IPV4_SRC])
> + saddr = nla_get_be32(tb[TCA_IPTUNNEL_ENC_IPV4_SRC]);
> + if (tb[TCA_IPTUNNEL_ENC_IPV4_DST])
> + daddr = nla_get_be32(tb[TCA_IPTUNNEL_ENC_IPV4_DST]);
> + if (tb[TCA_IPTUNNEL_ENC_KEY_ID])
> + key_id =
> key32_to_tunnel_id(nla_get_be32(tb[TCA_IPTUNNEL_ENC_KEY_ID]));
> +
> + if (!saddr || !daddr || !key_id) {
A zero tunnel ID is legit.
> + ret = -EINVAL;
> + goto err_out;
> + }
> +
> + metadata = iptunnel_alloc(t, saddr, daddr, key_id);
> + if (IS_ERR(metadata)) {
> + ret = PTR_ERR(metadata);
> + goto err_out;
> + }
> +
> + break;
> + default:
> + goto err_out;
> + }
> +
> + if (!exists) {
> + ret = tcf_hash_create(tn, parm->index, est, a,
> + &act_iptunnel_ops, bind, false);
> + if (ret)
> + return ret;
> +
> + ret = ACT_P_CREATED;
> + } else {
> + tcf_hash_release(*a, bind);
> + if (!ovr)
> + return -EEXIST;
> + }
> +
> + t = to_iptunnel(*a);
> +
> + spin_lock_bh(&t->tcf_lock);
> +
> + t->tcf_action = parm->action;
> +
> + t->tcft_action = encapdecap;
> + t->tcft_enc_metadata = metadata;
Although tcft_enc_metadata is not used in TCA_IPTUNNEL_ACT_DECAP, still
prefer to nullify it instead of initializing it to stack junk.
> +
> + spin_unlock_bh(&t->tcf_lock);
> +
> + if (ret == ACT_P_CREATED)
> + tcf_hash_insert(tn, *a);
> +
> + return ret;
In the (exists && ovr) case, 'ret' seems to be left as '-EINVAL' as was
initialized. Initialize 'ret' to zero instead.
> +
> +err_out:
> + if (exists)
> + tcf_hash_release(*a, bind);
> + return ret;
> +}
> +
