|
| 1 | +netfilter: bridge: replace physindev with physinif in nf_bridge_info |
| 2 | + |
| 3 | +jira LE-3201 |
| 4 | +cve CVE-2024-35839 |
| 5 | +Rebuild_History Non-Buildable kernel-rt-4.18.0-553.27.1.rt7.368.el8_10 |
| 6 | +commit-author Pavel Tikhomirov < [email protected]> |
| 7 | +commit 9874808878d9eed407e3977fd11fee49de1e1d86 |
| 8 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 9 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 10 | +ciq/ciq_backports/kernel-rt-4.18.0-553.27.1.rt7.368.el8_10/98748088.failed |
| 11 | + |
| 12 | +An skb can be added to a neigh->arp_queue while waiting for an arp |
| 13 | +reply. Where original skb's skb->dev can be different to neigh's |
| 14 | +neigh->dev. For instance in case of bridging dnated skb from one veth to |
| 15 | +another, the skb would be added to a neigh->arp_queue of the bridge. |
| 16 | + |
| 17 | +As skb->dev can be reset back to nf_bridge->physindev and used, and as |
| 18 | +there is no explicit mechanism that prevents this physindev from been |
| 19 | +freed under us (for instance neigh_flush_dev doesn't cleanup skbs from |
| 20 | +different device's neigh queue) we can crash on e.g. this stack: |
| 21 | + |
| 22 | +arp_process |
| 23 | + neigh_update |
| 24 | + skb = __skb_dequeue(&neigh->arp_queue) |
| 25 | + neigh_resolve_output(..., skb) |
| 26 | + ... |
| 27 | + br_nf_dev_xmit |
| 28 | + br_nf_pre_routing_finish_bridge_slow |
| 29 | + skb->dev = nf_bridge->physindev |
| 30 | + br_handle_frame_finish |
| 31 | + |
| 32 | +Let's use plain ifindex instead of net_device link. To peek into the |
| 33 | +original net_device we will use dev_get_by_index_rcu(). Thus either we |
| 34 | +get device and are safe to use it or we don't get it and drop skb. |
| 35 | + |
| 36 | +Fixes: c4e70a87d975 ("netfilter: bridge: rename br_netfilter.c to br_netfilter_hooks.c") |
| 37 | + Suggested-by: Florian Westphal < [email protected]> |
| 38 | + Signed-off-by: Pavel Tikhomirov < [email protected]> |
| 39 | + Signed-off-by: Pablo Neira Ayuso < [email protected]> |
| 40 | +(cherry picked from commit 9874808878d9eed407e3977fd11fee49de1e1d86) |
| 41 | + Signed-off-by: Jonathan Maple < [email protected]> |
| 42 | + |
| 43 | +# Conflicts: |
| 44 | +# net/bridge/br_netfilter_hooks.c |
| 45 | +diff --cc net/bridge/br_netfilter_hooks.c |
| 46 | +index 3935e9c04e6c,ed1720890757..000000000000 |
| 47 | +--- a/net/bridge/br_netfilter_hooks.c |
| 48 | ++++ b/net/bridge/br_netfilter_hooks.c |
| 49 | +@@@ -276,9 -277,19 +276,23 @@@ int br_nf_pre_routing_finish_bridge(str |
| 50 | + struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb); |
| 51 | + int ret; |
| 52 | + |
| 53 | +++<<<<<<< HEAD |
| 54 | + + if ((neigh->nud_state & NUD_CONNECTED) && neigh->hh.hh_len) { |
| 55 | +++======= |
| 56 | ++ if ((READ_ONCE(neigh->nud_state) & NUD_CONNECTED) && |
| 57 | ++ READ_ONCE(neigh->hh.hh_len)) { |
| 58 | ++ struct net_device *br_indev; |
| 59 | ++ |
| 60 | ++ br_indev = nf_bridge_get_physindev(skb, net); |
| 61 | ++ if (!br_indev) { |
| 62 | ++ neigh_release(neigh); |
| 63 | ++ goto free_skb; |
| 64 | ++ } |
| 65 | ++ |
| 66 | +++>>>>>>> 9874808878d9 (netfilter: bridge: replace physindev with physinif in nf_bridge_info) |
| 67 | + neigh_hh_bridge(&neigh->hh, skb); |
| 68 | +- skb->dev = nf_bridge->physindev; |
| 69 | ++ skb->dev = br_indev; |
| 70 | ++ |
| 71 | + ret = br_handle_frame_finish(net, sk, skb); |
| 72 | + } else { |
| 73 | + /* the neighbour function below overwrites the complete |
| 74 | +@@@ -450,8 -471,8 +470,13 @@@ struct net_device *setup_pre_routing(st |
| 75 | + } |
| 76 | + |
| 77 | + nf_bridge->in_prerouting = 1; |
| 78 | +++<<<<<<< HEAD |
| 79 | + + nf_bridge->physindev = skb->dev; |
| 80 | + + skb->dev = brnf_get_logical_dev(skb, skb->dev); |
| 81 | +++======= |
| 82 | ++ nf_bridge->physinif = skb->dev->ifindex; |
| 83 | ++ skb->dev = brnf_get_logical_dev(skb, skb->dev, net); |
| 84 | +++>>>>>>> 9874808878d9 (netfilter: bridge: replace physindev with physinif in nf_bridge_info) |
| 85 | + |
| 86 | + if (skb->protocol == htons(ETH_P_8021Q)) |
| 87 | + nf_bridge->orig_proto = BRNF_PROTO_8021Q; |
| 88 | +diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h |
| 89 | +index 25bf5871117c..515a4257bc51 100644 |
| 90 | +--- a/include/linux/netfilter_bridge.h |
| 91 | ++++ b/include/linux/netfilter_bridge.h |
| 92 | +@@ -35,7 +35,7 @@ static inline int nf_bridge_get_physinif(const struct sk_buff *skb) |
| 93 | + if (!nf_bridge) |
| 94 | + return 0; |
| 95 | + |
| 96 | +- return nf_bridge->physindev ? nf_bridge->physindev->ifindex : 0; |
| 97 | ++ return nf_bridge->physinif; |
| 98 | + } |
| 99 | + |
| 100 | + static inline int nf_bridge_get_physoutif(const struct sk_buff *skb) |
| 101 | +@@ -53,7 +53,7 @@ nf_bridge_get_physindev(const struct sk_buff *skb, struct net *net) |
| 102 | + { |
| 103 | + const struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb); |
| 104 | + |
| 105 | +- return nf_bridge ? nf_bridge->physindev : NULL; |
| 106 | ++ return nf_bridge ? dev_get_by_index_rcu(net, nf_bridge->physinif) : NULL; |
| 107 | + } |
| 108 | + |
| 109 | + static inline struct net_device * |
| 110 | +diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h |
| 111 | +index 6ca0c724a047..d920a4cb5f3f 100644 |
| 112 | +--- a/include/linux/skbuff.h |
| 113 | ++++ b/include/linux/skbuff.h |
| 114 | +@@ -268,7 +268,7 @@ struct nf_bridge_info { |
| 115 | + u8 in_prerouting:1; |
| 116 | + u8 bridged_dnat:1; |
| 117 | + __u16 frag_max_size; |
| 118 | +- struct net_device *physindev; |
| 119 | ++ int physinif; |
| 120 | + |
| 121 | + /* always valid & non-NULL from FORWARD on, for physdev match */ |
| 122 | + struct net_device *physoutdev; |
| 123 | +* Unmerged path net/bridge/br_netfilter_hooks.c |
| 124 | +diff --git a/net/bridge/br_netfilter_ipv6.c b/net/bridge/br_netfilter_ipv6.c |
| 125 | +index bec1423ac60f..2464e8134cfb 100644 |
| 126 | +--- a/net/bridge/br_netfilter_ipv6.c |
| 127 | ++++ b/net/bridge/br_netfilter_ipv6.c |
| 128 | +@@ -161,9 +161,15 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc |
| 129 | + { |
| 130 | + struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb); |
| 131 | + struct rtable *rt; |
| 132 | +- struct net_device *dev = skb->dev; |
| 133 | ++ struct net_device *dev = skb->dev, *br_indev; |
| 134 | + const struct nf_ipv6_ops *v6ops = nf_get_ipv6_ops(); |
| 135 | + |
| 136 | ++ br_indev = nf_bridge_get_physindev(skb, net); |
| 137 | ++ if (!br_indev) { |
| 138 | ++ kfree_skb(skb); |
| 139 | ++ return 0; |
| 140 | ++ } |
| 141 | ++ |
| 142 | + nf_bridge->frag_max_size = IP6CB(skb)->frag_max_size; |
| 143 | + |
| 144 | + if (nf_bridge->pkt_otherhost) { |
| 145 | +@@ -181,7 +187,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc |
| 146 | + } |
| 147 | + |
| 148 | + if (skb_dst(skb)->dev == dev) { |
| 149 | +- skb->dev = nf_bridge->physindev; |
| 150 | ++ skb->dev = br_indev; |
| 151 | + nf_bridge_update_protocol(skb); |
| 152 | + nf_bridge_push_encap_header(skb); |
| 153 | + br_nf_hook_thresh(NF_BR_PRE_ROUTING, |
| 154 | +@@ -192,7 +198,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc |
| 155 | + ether_addr_copy(eth_hdr(skb)->h_dest, dev->dev_addr); |
| 156 | + skb->pkt_type = PACKET_HOST; |
| 157 | + } else { |
| 158 | +- rt = bridge_parent_rtable(nf_bridge->physindev); |
| 159 | ++ rt = bridge_parent_rtable(br_indev); |
| 160 | + if (!rt) { |
| 161 | + kfree_skb(skb); |
| 162 | + return 0; |
| 163 | +@@ -201,7 +207,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc |
| 164 | + skb_dst_set_noref(skb, &rt->dst); |
| 165 | + } |
| 166 | + |
| 167 | +- skb->dev = nf_bridge->physindev; |
| 168 | ++ skb->dev = br_indev; |
| 169 | + nf_bridge_update_protocol(skb); |
| 170 | + nf_bridge_push_encap_header(skb); |
| 171 | + br_nf_hook_thresh(NF_BR_PRE_ROUTING, net, sk, skb, |
| 172 | +diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c |
| 173 | +index b74e5f5104aa..afb60a7aa62a 100644 |
| 174 | +--- a/net/ipv4/netfilter/nf_reject_ipv4.c |
| 175 | ++++ b/net/ipv4/netfilter/nf_reject_ipv4.c |
| 176 | +@@ -102,7 +102,6 @@ EXPORT_SYMBOL_GPL(nf_reject_ip_tcphdr_put); |
| 177 | + /* Send RST reply */ |
| 178 | + void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook) |
| 179 | + { |
| 180 | +- struct net_device *br_indev __maybe_unused; |
| 181 | + struct sk_buff *nskb; |
| 182 | + struct iphdr *niph; |
| 183 | + const struct tcphdr *oth; |
| 184 | +@@ -148,9 +147,13 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook) |
| 185 | + * build the eth header using the original destination's MAC as the |
| 186 | + * source, and send the RST packet directly. |
| 187 | + */ |
| 188 | +- br_indev = nf_bridge_get_physindev(oldskb, net); |
| 189 | +- if (br_indev) { |
| 190 | ++ if (nf_bridge_info_exists(oldskb)) { |
| 191 | + struct ethhdr *oeth = eth_hdr(oldskb); |
| 192 | ++ struct net_device *br_indev; |
| 193 | ++ |
| 194 | ++ br_indev = nf_bridge_get_physindev(oldskb, net); |
| 195 | ++ if (!br_indev) |
| 196 | ++ goto free_nskb; |
| 197 | + |
| 198 | + nskb->dev = br_indev; |
| 199 | + niph->tot_len = htons(nskb->len); |
| 200 | +diff --git a/net/ipv6/netfilter/nf_reject_ipv6.c b/net/ipv6/netfilter/nf_reject_ipv6.c |
| 201 | +index e0b2657c18b3..b4fffbe4d5cc 100644 |
| 202 | +--- a/net/ipv6/netfilter/nf_reject_ipv6.c |
| 203 | ++++ b/net/ipv6/netfilter/nf_reject_ipv6.c |
| 204 | +@@ -131,7 +131,6 @@ EXPORT_SYMBOL_GPL(nf_reject_ip6_tcphdr_put); |
| 205 | + |
| 206 | + void nf_send_reset6(struct net *net, struct sk_buff *oldskb, int hook) |
| 207 | + { |
| 208 | +- struct net_device *br_indev __maybe_unused; |
| 209 | + struct sk_buff *nskb; |
| 210 | + struct tcphdr _otcph; |
| 211 | + const struct tcphdr *otcph; |
| 212 | +@@ -198,9 +197,15 @@ void nf_send_reset6(struct net *net, struct sk_buff *oldskb, int hook) |
| 213 | + * build the eth header using the original destination's MAC as the |
| 214 | + * source, and send the RST packet directly. |
| 215 | + */ |
| 216 | +- br_indev = nf_bridge_get_physindev(oldskb, net); |
| 217 | +- if (br_indev) { |
| 218 | ++ if (nf_bridge_info_exists(oldskb)) { |
| 219 | + struct ethhdr *oeth = eth_hdr(oldskb); |
| 220 | ++ struct net_device *br_indev; |
| 221 | ++ |
| 222 | ++ br_indev = nf_bridge_get_physindev(oldskb, net); |
| 223 | ++ if (!br_indev) { |
| 224 | ++ kfree_skb(nskb); |
| 225 | ++ return; |
| 226 | ++ } |
| 227 | + |
| 228 | + nskb->dev = br_indev; |
| 229 | + nskb->protocol = htons(ETH_P_IPV6); |
0 commit comments