Commit b4565902 authored by Pavel Tikhomirov's avatar Pavel Tikhomirov Committed by Zhengchao Shao
Browse files

netfilter: bridge: replace physindev with physinif in nf_bridge_info 

mainline inclusion
from mainline-v6.8-rc1
commit 9874808878d9eed407e3977fd11fee49de1e1d86
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/I9Q9CH
CVE: CVE-2024-35839

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9874808878d9eed407e3977fd11fee49de1e1d86



--------------------------------

An skb can be added to a neigh->arp_queue while waiting for an arp
reply. Where original skb's skb->dev can be different to neigh's
neigh->dev. For instance in case of bridging dnated skb from one veth to
another, the skb would be added to a neigh->arp_queue of the bridge.

As skb->dev can be reset back to nf_bridge->physindev and used, and as
there is no explicit mechanism that prevents this physindev from been
freed under us (for instance neigh_flush_dev doesn't cleanup skbs from
different device's neigh queue) we can crash on e.g. this stack:

arp_process
  neigh_update
    skb = __skb_dequeue(&neigh->arp_queue)
      neigh_resolve_output(..., skb)
        ...
          br_nf_dev_xmit
            br_nf_pre_routing_finish_bridge_slow
              skb->dev = nf_bridge->physindev
              br_handle_frame_finish

Let's use plain ifindex instead of net_device link. To peek into the
original net_device we will use dev_get_by_index_rcu(). Thus either we
get device and are safe to use it or we don't get it and drop skb.

Fixes: c4e70a87 ("netfilter: bridge: rename br_netfilter.c to br_netfilter_hooks.c")
Suggested-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPavel Tikhomirov <ptikhomirov@virtuozzo.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>

Conflicts:
	 net/bridge/br_netfilter_hooks.c
[The conflict occurs because the commit b071af52("neighbour: annotate
lockless accesses to n->nud_state") is not merged]
Signed-off-by: default avatarZhengchao Shao <shaozhengchao@huawei.com>
parent f8d3cdf2

include/linux/netfilter_bridge.h

+2 −2
Original line number Diff line number Diff line
@@ -42,7 +42,7 @@ static inline int nf_bridge_get_physinif(const struct sk_buff *skb) 
	if (!nf_bridge)

		return 0;



	return nf_bridge->physindev ? nf_bridge->physindev->ifindex : 0;

	return nf_bridge->physinif;

}



static inline int nf_bridge_get_physoutif(const struct sk_buff *skb)
@@ -60,7 +60,7 @@ nf_bridge_get_physindev(const struct sk_buff *skb, struct net *net) 
{

	const struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);



	return nf_bridge ? nf_bridge->physindev : NULL;

	return nf_bridge ? dev_get_by_index_rcu(net, nf_bridge->physinif) : NULL;

}



static inline struct net_device *

include/linux/skbuff.h

+1 −1
Original line number Diff line number Diff line
@@ -263,7 +263,7 @@ struct nf_bridge_info { 
	u8			bridged_dnat:1;

	u8			sabotage_in_done:1;

	__u16			frag_max_size;

	struct net_device	*physindev;

	int			physinif;



	/* always valid & non-NULL from FORWARD on, for physdev match */

	struct net_device	*physoutdev;

net/bridge/br_netfilter_hooks.c

+33 −8
Original line number Diff line number Diff line
@@ -278,8 +278,16 @@ int br_nf_pre_routing_finish_bridge(struct net *net, struct sock *sk, struct sk_ 
		int ret;



		if ((neigh->nud_state & NUD_CONNECTED) && neigh->hh.hh_len) {

			struct net_device *br_indev;



			br_indev = nf_bridge_get_physindev(skb, net);

			if (!br_indev) {

				neigh_release(neigh);

				goto free_skb;

			}



			neigh_hh_bridge(&neigh->hh, skb);

			skb->dev = nf_bridge->physindev;

			skb->dev = br_indev;

			ret = br_handle_frame_finish(net, sk, skb);

		} else {

			/* the neighbour function below overwrites the complete
@@ -351,12 +359,18 @@ br_nf_ipv4_daddr_was_changed(const struct sk_buff *skb, 
 */

static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_buff *skb)

{

	struct net_device *dev = skb->dev;

	struct net_device *dev = skb->dev, *br_indev;

	struct iphdr *iph = ip_hdr(skb);

	struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);

	struct rtable *rt;

	int err;



	br_indev = nf_bridge_get_physindev(skb, net);

	if (!br_indev) {

		kfree_skb(skb);

		return 0;

	}



	nf_bridge->frag_max_size = IPCB(skb)->frag_max_size;



	if (nf_bridge->pkt_otherhost) {
@@ -396,7 +410,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_ 
		} else {

			if (skb_dst(skb)->dev == dev) {

bridged_dnat:

				skb->dev = nf_bridge->physindev;

				skb->dev = br_indev;

				nf_bridge_update_protocol(skb);

				nf_bridge_push_encap_header(skb);

				br_nf_hook_thresh(NF_BR_PRE_ROUTING,
@@ -409,7 +423,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_ 
			skb->pkt_type = PACKET_HOST;

		}

	} else {

		rt = bridge_parent_rtable(nf_bridge->physindev);

		rt = bridge_parent_rtable(br_indev);

		if (!rt) {

			kfree_skb(skb);

			return 0;
@@ -418,7 +432,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_ 
		skb_dst_set_noref(skb, &rt->dst);

	}



	skb->dev = nf_bridge->physindev;

	skb->dev = br_indev;

	nf_bridge_update_protocol(skb);

	nf_bridge_push_encap_header(skb);

	br_nf_hook_thresh(NF_BR_PRE_ROUTING, net, sk, skb, skb->dev, NULL,
@@ -455,7 +469,7 @@ struct net_device *setup_pre_routing(struct sk_buff *skb, const struct net *net) 
	}



	nf_bridge->in_prerouting = 1;

	nf_bridge->physindev = skb->dev;

	nf_bridge->physinif = skb->dev->ifindex;

	skb->dev = brnf_get_logical_dev(skb, skb->dev, net);



	if (skb->protocol == htons(ETH_P_8021Q))
@@ -552,7 +566,11 @@ static int br_nf_forward_finish(struct net *net, struct sock *sk, struct sk_buff 
		if (skb->protocol == htons(ETH_P_IPV6))

			nf_bridge->frag_max_size = IP6CB(skb)->frag_max_size;



		in = nf_bridge->physindev;

		in = nf_bridge_get_physindev(skb, net);

		if (!in) {

			kfree_skb(skb);

			return 0;

		}

		if (nf_bridge->pkt_otherhost) {

			skb->pkt_type = PACKET_OTHERHOST;

			nf_bridge->pkt_otherhost = false;
@@ -896,6 +914,13 @@ static unsigned int ip_sabotage_in(void *priv, 
static void br_nf_pre_routing_finish_bridge_slow(struct sk_buff *skb)

{

	struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);

	struct net_device *br_indev;



	br_indev = nf_bridge_get_physindev(skb, dev_net(skb->dev));

	if (!br_indev) {

		kfree_skb(skb);

		return;

	}



	skb_pull(skb, ETH_HLEN);

	nf_bridge->bridged_dnat = 0;
@@ -905,7 +930,7 @@ static void br_nf_pre_routing_finish_bridge_slow(struct sk_buff *skb) 
	skb_copy_to_linear_data_offset(skb, -(ETH_HLEN - ETH_ALEN),

				       nf_bridge->neigh_header,

				       ETH_HLEN - ETH_ALEN);

	skb->dev = nf_bridge->physindev;

	skb->dev = br_indev;



	nf_bridge->physoutdev = NULL;

	br_handle_frame_finish(dev_net(skb->dev), NULL, skb);

net/bridge/br_netfilter_ipv6.c

+10 −4
Original line number Diff line number Diff line
@@ -161,9 +161,15 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc 
{

	struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);

	struct rtable *rt;

	struct net_device *dev = skb->dev;

	struct net_device *dev = skb->dev, *br_indev;

	const struct nf_ipv6_ops *v6ops = nf_get_ipv6_ops();



	br_indev = nf_bridge_get_physindev(skb, net);

	if (!br_indev) {

		kfree_skb(skb);

		return 0;

	}



	nf_bridge->frag_max_size = IP6CB(skb)->frag_max_size;



	if (nf_bridge->pkt_otherhost) {
@@ -181,7 +187,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc 
		}



		if (skb_dst(skb)->dev == dev) {

			skb->dev = nf_bridge->physindev;

			skb->dev = br_indev;

			nf_bridge_update_protocol(skb);

			nf_bridge_push_encap_header(skb);

			br_nf_hook_thresh(NF_BR_PRE_ROUTING,
@@ -192,7 +198,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc 
		ether_addr_copy(eth_hdr(skb)->h_dest, dev->dev_addr);

		skb->pkt_type = PACKET_HOST;

	} else {

		rt = bridge_parent_rtable(nf_bridge->physindev);

		rt = bridge_parent_rtable(br_indev);

		if (!rt) {

			kfree_skb(skb);

			return 0;
@@ -201,7 +207,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc 
		skb_dst_set_noref(skb, &rt->dst);

	}



	skb->dev = nf_bridge->physindev;

	skb->dev = br_indev;

	nf_bridge_update_protocol(skb);

	nf_bridge_push_encap_header(skb);

	br_nf_hook_thresh(NF_BR_PRE_ROUTING, net, sk, skb,

net/ipv4/netfilter/nf_reject_ipv4.c

+6 −3
Original line number Diff line number Diff line
@@ -115,7 +115,6 @@ static int nf_reject_fill_skb_dst(struct sk_buff *skb_in) 
void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,

		   int hook)

{

	struct net_device *br_indev __maybe_unused;

	struct sk_buff *nskb;

	struct iphdr *niph;

	const struct tcphdr *oth;
@@ -163,9 +162,13 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb, 
	 * build the eth header using the original destination's MAC as the

	 * source, and send the RST packet directly.

	 */

	br_indev = nf_bridge_get_physindev(oldskb, net);

	if (br_indev) {

	if (nf_bridge_info_exists(oldskb)) {

		struct ethhdr *oeth = eth_hdr(oldskb);

		struct net_device *br_indev;



		br_indev = nf_bridge_get_physindev(oldskb, net);

		if (!br_indev)

			goto free_nskb;



		nskb->dev = br_indev;

		niph->tot_len = htons(nskb->len);