Skip to content

Commit e741bc2

Browse files
zampierilucaszampierilucas
committed
Merge: kernel: netfilter: bridge: replace physindev with physinif in nf_bridge_info
MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/4537 JIRA: https://issues.redhat.com/browse/RHEL-37040 JIRA: https://issues.redhat.com/browse/RHEL-37041 CVE: CVE-2024-35839 nf_bridge->physindev stores dev pointer without holding reference, so if skb gets queued somewhere and device disappears further access results in UaF. Store device ifindex instead and re-lookup, its less intrusive and error prone than adding refcounting. Signed-off-by: Florian Westphal <fwestpha@redhat.com> Approved-by: Antoine Tenart <atenart@redhat.com> Approved-by: Hangbin Liu <haliu@redhat.com> Approved-by: CKI KWF Bot <cki-ci-bot+kwf-gitlab-com@redhat.com> Merged-by: Lucas Zampieri <lzampier@redhat.com>
2 parents b42e692 + 6fa5dcc commit e741bc2

File tree

11 files changed

+80
-41
lines changed

11 files changed

+80
-41
lines changed

‎include/linux/netfilter_bridge.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ static inline int nf_bridge_get_physinif(const struct sk_buff *skb)
4242
if (!nf_bridge)
4343
return 0;
4444

45-
return nf_bridge->physindev ? nf_bridge->physindev->ifindex : 0;
45+
return nf_bridge->physinif;
4646
}
4747

4848
static inline int nf_bridge_get_physoutif(const struct sk_buff *skb)
@@ -56,11 +56,11 @@ static inline int nf_bridge_get_physoutif(const struct sk_buff *skb)
5656
}
5757

5858
static inline struct net_device *
59-
nf_bridge_get_physindev(const struct sk_buff *skb)
59+
nf_bridge_get_physindev(const struct sk_buff *skb, struct net *net)
6060
{
6161
const struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
6262

63-
return nf_bridge ? nf_bridge->physindev : NULL;
63+
return nf_bridge ? dev_get_by_index_rcu(net, nf_bridge->physinif) : NULL;
6464
}
6565

6666
static inline struct net_device *

‎include/linux/skbuff.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ struct nf_bridge_info {
272272
u8 bridged_dnat:1;
273273
u8 sabotage_in_done:1;
274274
__u16 frag_max_size;
275-
struct net_device *physindev;
275+
int physinif;
276276

277277
/* always valid & non-NULL from FORWARD on, for physdev match */
278278
struct net_device *physoutdev;

‎net/bridge/br_netfilter_hooks.c‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,17 @@ int br_nf_pre_routing_finish_bridge(struct net *net, struct sock *sk, struct sk_
279279

280280
if ((READ_ONCE(neigh->nud_state) & NUD_CONNECTED) &&
281281
READ_ONCE(neigh->hh.hh_len)) {
282+
struct net_device *br_indev;
283+
284+
br_indev = nf_bridge_get_physindev(skb, net);
285+
if (!br_indev) {
286+
neigh_release(neigh);
287+
goto free_skb;
288+
}
289+
282290
neigh_hh_bridge(&neigh->hh, skb);
283-
skb->dev = nf_bridge->physindev;
291+
skb->dev = br_indev;
292+
284293
ret = br_handle_frame_finish(net, sk, skb);
285294
} else {
286295
/* the neighbour function below overwrites the complete
@@ -352,12 +361,18 @@ br_nf_ipv4_daddr_was_changed(const struct sk_buff *skb,
352361
*/
353362
static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
354363
{
355-
struct net_device *dev = skb->dev;
364+
struct net_device *dev = skb->dev, *br_indev;
356365
struct iphdr *iph = ip_hdr(skb);
357366
struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
358367
struct rtable *rt;
359368
int err;
360369

370+
br_indev = nf_bridge_get_physindev(skb, net);
371+
if (!br_indev) {
372+
kfree_skb(skb);
373+
return 0;
374+
}
375+
361376
nf_bridge->frag_max_size = IPCB(skb)->frag_max_size;
362377

363378
if (nf_bridge->pkt_otherhost) {
@@ -397,7 +412,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
397412
} else {
398413
if (skb_dst(skb)->dev == dev) {
399414
bridged_dnat:
400-
skb->dev = nf_bridge->physindev;
415+
skb->dev = br_indev;
401416
nf_bridge_update_protocol(skb);
402417
nf_bridge_push_encap_header(skb);
403418
br_nf_hook_thresh(NF_BR_PRE_ROUTING,
@@ -410,7 +425,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
410425
skb->pkt_type = PACKET_HOST;
411426
}
412427
} else {
413-
rt = bridge_parent_rtable(nf_bridge->physindev);
428+
rt = bridge_parent_rtable(br_indev);
414429
if (!rt) {
415430
kfree_skb(skb);
416431
return 0;
@@ -419,7 +434,7 @@ static int br_nf_pre_routing_finish(struct net *net, struct sock *sk, struct sk_
419434
skb_dst_set_noref(skb, &rt->dst);
420435
}
421436

422-
skb->dev = nf_bridge->physindev;
437+
skb->dev = br_indev;
423438
nf_bridge_update_protocol(skb);
424439
nf_bridge_push_encap_header(skb);
425440
br_nf_hook_thresh(NF_BR_PRE_ROUTING, net, sk, skb, skb->dev, NULL,
@@ -456,7 +471,7 @@ struct net_device *setup_pre_routing(struct sk_buff *skb, const struct net *net)
456471
}
457472

458473
nf_bridge->in_prerouting = 1;
459-
nf_bridge->physindev = skb->dev;
474+
nf_bridge->physinif = skb->dev->ifindex;
460475
skb->dev = brnf_get_logical_dev(skb, skb->dev, net);
461476

462477
if (skb->protocol == htons(ETH_P_8021Q))
@@ -553,7 +568,11 @@ static int br_nf_forward_finish(struct net *net, struct sock *sk, struct sk_buff
553568
if (skb->protocol == htons(ETH_P_IPV6))
554569
nf_bridge->frag_max_size = IP6CB(skb)->frag_max_size;
555570

556-
in = nf_bridge->physindev;
571+
in = nf_bridge_get_physindev(skb, net);
572+
if (!in) {
573+
kfree_skb(skb);
574+
return 0;
575+
}
557576
if (nf_bridge->pkt_otherhost) {
558577
skb->pkt_type = PACKET_OTHERHOST;
559578
nf_bridge->pkt_otherhost = false;
@@ -899,6 +918,13 @@ static unsigned int ip_sabotage_in(void *priv,
899918
static void br_nf_pre_routing_finish_bridge_slow(struct sk_buff *skb)
900919
{
901920
struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
921+
struct net_device *br_indev;
922+
923+
br_indev = nf_bridge_get_physindev(skb, dev_net(skb->dev));
924+
if (!br_indev) {
925+
kfree_skb(skb);
926+
return;
927+
}
902928

903929
skb_pull(skb, ETH_HLEN);
904930
nf_bridge->bridged_dnat = 0;
@@ -908,7 +934,7 @@ static void br_nf_pre_routing_finish_bridge_slow(struct sk_buff *skb)
908934
skb_copy_to_linear_data_offset(skb, -(ETH_HLEN - ETH_ALEN),
909935
nf_bridge->neigh_header,
910936
ETH_HLEN - ETH_ALEN);
911-
skb->dev = nf_bridge->physindev;
937+
skb->dev = br_indev;
912938

913939
nf_bridge->physoutdev = NULL;
914940
br_handle_frame_finish(dev_net(skb->dev), NULL, skb);

‎net/bridge/br_netfilter_ipv6.c‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,15 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
102102
{
103103
struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
104104
struct rtable *rt;
105-
struct net_device *dev = skb->dev;
105+
struct net_device *dev = skb->dev, *br_indev;
106106
const struct nf_ipv6_ops *v6ops = nf_get_ipv6_ops();
107107

108+
br_indev = nf_bridge_get_physindev(skb, net);
109+
if (!br_indev) {
110+
kfree_skb(skb);
111+
return 0;
112+
}
113+
108114
nf_bridge->frag_max_size = IP6CB(skb)->frag_max_size;
109115

110116
if (nf_bridge->pkt_otherhost) {
@@ -122,7 +128,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
122128
}
123129

124130
if (skb_dst(skb)->dev == dev) {
125-
skb->dev = nf_bridge->physindev;
131+
skb->dev = br_indev;
126132
nf_bridge_update_protocol(skb);
127133
nf_bridge_push_encap_header(skb);
128134
br_nf_hook_thresh(NF_BR_PRE_ROUTING,
@@ -133,7 +139,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
133139
ether_addr_copy(eth_hdr(skb)->h_dest, dev->dev_addr);
134140
skb->pkt_type = PACKET_HOST;
135141
} else {
136-
rt = bridge_parent_rtable(nf_bridge->physindev);
142+
rt = bridge_parent_rtable(br_indev);
137143
if (!rt) {
138144
kfree_skb(skb);
139145
return 0;
@@ -142,7 +148,7 @@ static int br_nf_pre_routing_finish_ipv6(struct net *net, struct sock *sk, struc
142148
skb_dst_set_noref(skb, &rt->dst);
143149
}
144150

145-
skb->dev = nf_bridge->physindev;
151+
skb->dev = br_indev;
146152
nf_bridge_update_protocol(skb);
147153
nf_bridge_push_encap_header(skb);
148154
br_nf_hook_thresh(NF_BR_PRE_ROUTING, net, sk, skb,

‎net/ipv4/netfilter/nf_reject_ipv4.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,6 @@ static int nf_reject_fill_skb_dst(struct sk_buff *skb_in)
237237
void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
238238
int hook)
239239
{
240-
struct net_device *br_indev __maybe_unused;
241240
struct sk_buff *nskb;
242241
struct iphdr *niph;
243242
const struct tcphdr *oth;
@@ -286,9 +285,13 @@ void nf_send_reset(struct net *net, struct sock *sk, struct sk_buff *oldskb,
286285
* build the eth header using the original destination's MAC as the
287286
* source, and send the RST packet directly.
288287
*/
289-
br_indev = nf_bridge_get_physindev(oldskb);
290-
if (br_indev) {
288+
if (nf_bridge_info_exists(oldskb)) {
291289
struct ethhdr *oeth = eth_hdr(oldskb);
290+
struct net_device *br_indev;
291+
292+
br_indev = nf_bridge_get_physindev(oldskb, net);
293+
if (!br_indev)
294+
goto free_nskb;
292295

293296
nskb->dev = br_indev;
294297
niph->tot_len = htons(nskb->len);

‎net/ipv6/netfilter/nf_reject_ipv6.c‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@ static int nf_reject6_fill_skb_dst(struct sk_buff *skb_in)
278278
void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
279279
int hook)
280280
{
281-
struct net_device *br_indev __maybe_unused;
282281
struct sk_buff *nskb;
283282
struct tcphdr _otcph;
284283
const struct tcphdr *otcph;
@@ -353,9 +352,15 @@ void nf_send_reset6(struct net *net, struct sock *sk, struct sk_buff *oldskb,
353352
* build the eth header using the original destination's MAC as the
354353
* source, and send the RST packet directly.
355354
*/
356-
br_indev = nf_bridge_get_physindev(oldskb);
357-
if (br_indev) {
355+
if (nf_bridge_info_exists(oldskb)) {
358356
struct ethhdr *oeth = eth_hdr(oldskb);
357+
struct net_device *br_indev;
358+
359+
br_indev = nf_bridge_get_physindev(oldskb, net);
360+
if (!br_indev) {
361+
kfree_skb(nskb);
362+
return;
363+
}
359364

360365
nskb->dev = br_indev;
361366
nskb->protocol = htons(ETH_P_IPV6);

‎net/netfilter/ipset/ip_set_hash_netiface.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ hash_netiface4_data_next(struct hash_netiface4_elem *next,
138138
#include "ip_set_hash_gen.h"
139139

140140
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
141-
static const char *get_physindev_name(const struct sk_buff *skb)
141+
static const char *get_physindev_name(const struct sk_buff *skb, struct net *net)
142142
{
143-
struct net_device *dev = nf_bridge_get_physindev(skb);
143+
struct net_device *dev = nf_bridge_get_physindev(skb, net);
144144

145145
return dev ? dev->name : NULL;
146146
}
@@ -177,7 +177,7 @@ hash_netiface4_kadt(struct ip_set *set, const struct sk_buff *skb,
177177

178178
if (opt->cmdflags & IPSET_FLAG_PHYSDEV) {
179179
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
180-
const char *eiface = SRCDIR ? get_physindev_name(skb) :
180+
const char *eiface = SRCDIR ? get_physindev_name(skb, xt_net(par)) :
181181
get_physoutdev_name(skb);
182182

183183
if (!eiface)
@@ -395,7 +395,7 @@ hash_netiface6_kadt(struct ip_set *set, const struct sk_buff *skb,
395395

396396
if (opt->cmdflags & IPSET_FLAG_PHYSDEV) {
397397
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
398-
const char *eiface = SRCDIR ? get_physindev_name(skb) :
398+
const char *eiface = SRCDIR ? get_physindev_name(skb, xt_net(par)) :
399399
get_physoutdev_name(skb);
400400

401401
if (!eiface)

‎net/netfilter/nf_log_syslog.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ nf_log_dump_packet_common(struct nf_log_buf *m, u8 pf,
111111
unsigned int hooknum, const struct sk_buff *skb,
112112
const struct net_device *in,
113113
const struct net_device *out,
114-
const struct nf_loginfo *loginfo, const char *prefix)
114+
const struct nf_loginfo *loginfo, const char *prefix,
115+
struct net *net)
115116
{
116117
const struct net_device *physoutdev __maybe_unused;
117118
const struct net_device *physindev __maybe_unused;
@@ -121,7 +122,7 @@ nf_log_dump_packet_common(struct nf_log_buf *m, u8 pf,
121122
in ? in->name : "",
122123
out ? out->name : "");
123124
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
124-
physindev = nf_bridge_get_physindev(skb);
125+
physindev = nf_bridge_get_physindev(skb, net);
125126
if (physindev && in != physindev)
126127
nf_log_buf_add(m, "PHYSIN=%s ", physindev->name);
127128
physoutdev = nf_bridge_get_physoutdev(skb);
@@ -148,7 +149,7 @@ static void nf_log_arp_packet(struct net *net, u_int8_t pf,
148149
loginfo = &default_loginfo;
149150

150151
nf_log_dump_packet_common(m, pf, hooknum, skb, in, out, loginfo,
151-
prefix);
152+
prefix, net);
152153
dump_arp_packet(m, loginfo, skb, skb_network_offset(skb));
153154

154155
nf_log_buf_close(m);
@@ -845,7 +846,7 @@ static void nf_log_ip_packet(struct net *net, u_int8_t pf,
845846
loginfo = &default_loginfo;
846847

847848
nf_log_dump_packet_common(m, pf, hooknum, skb, in,
848-
out, loginfo, prefix);
849+
out, loginfo, prefix, net);
849850

850851
if (in)
851852
dump_mac_header(m, loginfo, skb);
@@ -880,7 +881,7 @@ static void nf_log_ip6_packet(struct net *net, u_int8_t pf,
880881
loginfo = &default_loginfo;
881882

882883
nf_log_dump_packet_common(m, pf, hooknum, skb, in, out,
883-
loginfo, prefix);
884+
loginfo, prefix, net);
884885

885886
if (in)
886887
dump_mac_header(m, loginfo, skb);
@@ -916,7 +917,7 @@ static void nf_log_unknown_packet(struct net *net, u_int8_t pf,
916917
loginfo = &default_loginfo;
917918

918919
nf_log_dump_packet_common(m, pf, hooknum, skb, in, out, loginfo,
919-
prefix);
920+
prefix, net);
920921

921922
dump_mac_header(m, loginfo, skb);
922923

‎net/netfilter/nf_queue.c‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,9 @@ static void __nf_queue_entry_init_physdevs(struct nf_queue_entry *entry)
8282
{
8383
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
8484
const struct sk_buff *skb = entry->skb;
85-
struct nf_bridge_info *nf_bridge;
8685

87-
nf_bridge = nf_bridge_info_get(skb);
88-
if (nf_bridge) {
89-
entry->physin = nf_bridge_get_physindev(skb);
86+
if (nf_bridge_info_exists(skb)) {
87+
entry->physin = nf_bridge_get_physindev(skb, entry->state.net);
9088
entry->physout = nf_bridge_get_physoutdev(skb);
9189
} else {
9290
entry->physin = NULL;

‎net/netfilter/nfnetlink_log.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -499,18 +499,18 @@ __build_packet_message(struct nfnl_log_net *log,
499499
htonl(br_port_get_rcu(indev)->br->dev->ifindex)))
500500
goto nla_put_failure;
501501
} else {
502-
struct net_device *physindev;
502+
int physinif;
503503

504504
/* Case 2: indev is bridge group, we need to look for
505505
* physical device (when called from ipv4) */
506506
if (nla_put_be32(inst->skb, NFULA_IFINDEX_INDEV,
507507
htonl(indev->ifindex)))
508508
goto nla_put_failure;
509509

510-
physindev = nf_bridge_get_physindev(skb);
511-
if (physindev &&
510+
physinif = nf_bridge_get_physinif(skb);
511+
if (physinif &&
512512
nla_put_be32(inst->skb, NFULA_IFINDEX_PHYSINDEV,
513-
htonl(physindev->ifindex)))
513+
htonl(physinif)))
514514
goto nla_put_failure;
515515
}
516516
#endif

0 commit comments

Comments
 (0)