| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5628B94A.5010903@brocade.com>
Date: Thu, 22 Oct 2015 11:24:10 +0100
From: Robert Shearman <rshearma@...cade.com>
To: Roopa Prabhu <roopa@...ulusnetworks.com>, <davem@...emloft.net>
CC: <netdev@...r.kernel.org>, <ebiederm@...ssion.com>
Subject: Re: [PATCH net-next v4 1/2] mpls: multipath route support
On 19/10/15 02:20, Roopa Prabhu wrote:
> @@ -159,6 +137,7 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
> struct net *net = dev_net(dev);
> struct mpls_shim_hdr *hdr;
> struct mpls_route *rt;
> + struct mpls_nh *nh;
> struct mpls_entry_decoded dec;
> struct net_device *out_dev;
> struct mpls_dev *mdev;
> @@ -166,6 +145,7 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
> unsigned int new_header_size;
> unsigned int mtu;
> int err;
> + int nhidx;
nhidx looks to be unused. Remove?
>
> /* Careful this entire function runs inside of an rcu critical section */
>
> @@ -196,8 +176,12 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
> if (!rt)
> goto drop;
>
> + nh = mpls_select_multipath(rt);
> + if (!nh)
> + goto drop;
> +
> /* Find the output device */
> - out_dev = rcu_dereference(rt->rt_dev);
> + out_dev = rcu_dereference(nh->nh_dev);
> if (!mpls_output_possible(out_dev))
> goto drop;
>
> @@ -212,7 +196,7 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
> dec.ttl -= 1;
>
> /* Verify the destination can hold the packet */
> - new_header_size = mpls_rt_header_size(rt);
> + new_header_size = mpls_nh_header_size(nh);
> mtu = mpls_dev_mtu(out_dev);
> if (mpls_pkt_too_big(skb, mtu - new_header_size))
> goto drop;
> @@ -240,13 +224,14 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
> /* Push the new labels */
> hdr = mpls_hdr(skb);
> bos = dec.bos;
> - for (i = rt->rt_labels - 1; i >= 0; i--) {
> - hdr[i] = mpls_entry_encode(rt->rt_label[i], dec.ttl, 0, bos);
> + for (i = nh->nh_labels - 1; i >= 0; i--) {
> + hdr[i] = mpls_entry_encode(nh->nh_label[i],
> + dec.ttl, 0, bos);
> bos = false;
> }
> }
>
> - err = neigh_xmit(rt->rt_via_table, out_dev, rt->rt_via, skb);
> + err = neigh_xmit(nh->nh_via_table, out_dev, nh->nh_via, skb);
> if (err)
> net_dbg_ratelimited("%s: packet transmission failed: %d\n",
> __func__, err);
> return dev;
> }
>
> +static int mpls_nh_assign_dev(struct net *net, struct mpls_nh *nh, int oif)
> +{
> + struct net_device *dev = NULL;
> + int err = -ENODEV;
> +
> + dev = find_outdev(net, nh, oif);
> + if (IS_ERR(dev)) {
> + err = PTR_ERR(dev);
> + dev = NULL;
> + goto errout;
> + }
> +
> + /* Ensure this is a supported device */
> + err = -EINVAL;
> + if (!mpls_dev_get(dev))
> + goto errout;
> +
> + /* For now just support ethernet devices */
> + if ((dev->type != ARPHRD_ETHER) && (dev->type != ARPHRD_LOOPBACK))
> + goto errout;
> +
> + RCU_INIT_POINTER(nh->nh_dev, dev);
> + dev_put(dev);
If it's safe to not keep a reference to dev (i.e. because the device
cannot go away while a netlink message is being processed), then why not
just change find_outdev to use __dev_get_by_index? If the dev can go
away during this, then we could end up adding an mpls route that points
to a freed interface, because until we've added the route to the label
table mpls_ifdown won't find it.
> +
> + return 0;
> +
> +errout:
> + if (dev)
> + dev_put(dev);
> + return err;
> +}
> +
> }
> EXPORT_SYMBOL_GPL(nla_get_labels);
>
> +int nla_get_via(const struct nlattr *nla, u8 *via_alen,
> + u8 *via_table, u8 via_addr[])
> +{
> + struct rtvia *via = nla_data(nla);
> + int err = -EINVAL;
> + u8 alen;
This should be an int to avoid nexthop address lengths > 255 wrapping
and possibly accepted instead of rejected.
> +
> + if (nla_len(nla) < offsetof(struct rtvia, rtvia_addr))
> + goto errout;
> + alen = nla_len(nla) -
> + offsetof(struct rtvia, rtvia_addr);
> + if (alen > MAX_VIA_ALEN)
> + goto errout;
> +
> + /* Validate the address family */
> + switch(via->rtvia_family) {
> + case AF_PACKET:
> + *via_table = NEIGH_LINK_TABLE;
> + break;
> + case AF_INET:
> + *via_table = NEIGH_ARP_TABLE;
> + if (alen != 4)
> + goto errout;
> + break;
> + case AF_INET6:
> + *via_table = NEIGH_ND_TABLE;
> + if (alen != 16)
> + goto errout;
> + break;
> + default:
> + /* Unsupported address family */
> + goto errout;
> + }
> +
> + memcpy(via_addr, via->rtvia_addr, alen);
> + *via_alen = alen;
> + err = 0;
> +
> +errout:
> + return err;
> +}
> +
> static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh,
> struct mpls_route_config *cfg)
> {
Otherwise, looks good.
Thanks,
Rob
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists