On Tue, Mar 26, 2019 at 08:29:32PM -0700, David Ahern wrote:
> +int fib_nh_init(struct net *net, struct fib_nh *nh,
> + struct fib_config *cfg, int nh_weight,
> + struct netlink_ext_ack *extack)
> +{
> + int err = -ENOMEM;
> +
> + nh->nh_pcpu_rth_output = alloc_percpu(struct rtable __rcu *);
> + if (!nh->nh_pcpu_rth_output)
> + goto failure;
> +
> + if (cfg->fc_encap) {
> + struct lwtunnel_state *lwtstate;
> +
> + err = -EINVAL;
> + if (cfg->fc_encap_type == LWTUNNEL_ENCAP_NONE) {
> + NL_SET_ERR_MSG(extack, "LWT encap type not specified");
> + goto failure;
This is very confusing and probably error-prone. You call
alloc_percpu(), but don't free it in the error path and instead rely on
the call to free_fib_info() in the error path of fib_create_info().
Better to free it here and NULL-ify the pointer. Then the call to
rt_fibinfo_free_cpus() in free_fib_info_rcu() is basically a NOP.
> + }
> + err = lwtunnel_build_state(cfg->fc_encap_type,
> + cfg->fc_encap, AF_INET, cfg,
> + &lwtstate, extack);
> + if (err)
> + goto failure;
> +
> + nh->nh_lwtstate = lwtstate_get(lwtstate);
> + }
> +
> + nh->nh_oif = cfg->fc_oif;
> + nh->nh_gw = cfg->fc_gw;
> + nh->nh_flags = cfg->fc_flags;
> +
> +#ifdef CONFIG_IP_ROUTE_CLASSID
> + nh->nh_tclassid = cfg->fc_flow;
> + if (nh->nh_tclassid)
> + net->ipv4.fib_num_tclassid_users++;
> +#endif
> +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> + nh->nh_weight = nh_weight;
> +#endif
> + err = 0;
> +
> +failure:
> + return err;
> +}
