Skip to content

Commit c6dd1aa

Browse files
fblaesekuba-moo
authored andcommitted
icmp: fix icmp_ndo_send address translation for reply direction
The icmp_ndo_send function was originally introduced to ensure proper rate limiting when icmp_send is called by a network device driver, where the packet's source address may have already been transformed by SNAT. However, the original implementation only considers the IP_CT_DIR_ORIGINAL direction for SNAT and always replaced the packet's source address with that of the original-direction tuple. This causes two problems: 1. For SNAT: Reply-direction packets were incorrectly translated using the source address of the CT original direction, even though no translation is required. 2. For DNAT: Reply-direction packets were not handled at all. In DNAT, the original direction's destination is translated. Therefore, in the reply direction the source address must be set to the reply-direction source, so rate limiting works as intended. Fix this by using the connection direction to select the correct tuple for source address translation, and adjust the pre-checks to handle reply-direction packets in case of DNAT. Additionally, wrap the `ct->status` access in READ_ONCE(). This avoids possible KCSAN reports about concurrent updates to `ct->status`. Fixes: 0b41713 ("icmp: introduce helper for nat'd source address in network device context") Signed-off-by: Fabian Bläse <[email protected]> Cc: Jason A. Donenfeld <[email protected]> Reviewed-by: Florian Westphal <[email protected]> Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 7000f4f commit c6dd1aa

File tree

2 files changed

+8
-4
lines changed

2 files changed

+8
-4
lines changed

net/ipv4/icmp.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -799,11 +799,12 @@ void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
799799
struct sk_buff *cloned_skb = NULL;
800800
struct ip_options opts = { 0 };
801801
enum ip_conntrack_info ctinfo;
802+
enum ip_conntrack_dir dir;
802803
struct nf_conn *ct;
803804
__be32 orig_ip;
804805

805806
ct = nf_ct_get(skb_in, &ctinfo);
806-
if (!ct || !(ct->status & IPS_SRC_NAT)) {
807+
if (!ct || !(READ_ONCE(ct->status) & IPS_NAT_MASK)) {
807808
__icmp_send(skb_in, type, code, info, &opts);
808809
return;
809810
}
@@ -818,7 +819,8 @@ void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
818819
goto out;
819820

820821
orig_ip = ip_hdr(skb_in)->saddr;
821-
ip_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.ip;
822+
dir = CTINFO2DIR(ctinfo);
823+
ip_hdr(skb_in)->saddr = ct->tuplehash[dir].tuple.src.u3.ip;
822824
__icmp_send(skb_in, type, code, info, &opts);
823825
ip_hdr(skb_in)->saddr = orig_ip;
824826
out:

net/ipv6/ip6_icmp.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,12 @@ void icmpv6_ndo_send(struct sk_buff *skb_in, u8 type, u8 code, __u32 info)
5454
struct inet6_skb_parm parm = { 0 };
5555
struct sk_buff *cloned_skb = NULL;
5656
enum ip_conntrack_info ctinfo;
57+
enum ip_conntrack_dir dir;
5758
struct in6_addr orig_ip;
5859
struct nf_conn *ct;
5960

6061
ct = nf_ct_get(skb_in, &ctinfo);
61-
if (!ct || !(ct->status & IPS_SRC_NAT)) {
62+
if (!ct || !(READ_ONCE(ct->status) & IPS_NAT_MASK)) {
6263
__icmpv6_send(skb_in, type, code, info, &parm);
6364
return;
6465
}
@@ -73,7 +74,8 @@ void icmpv6_ndo_send(struct sk_buff *skb_in, u8 type, u8 code, __u32 info)
7374
goto out;
7475

7576
orig_ip = ipv6_hdr(skb_in)->saddr;
76-
ipv6_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.in6;
77+
dir = CTINFO2DIR(ctinfo);
78+
ipv6_hdr(skb_in)->saddr = ct->tuplehash[dir].tuple.src.u3.in6;
7779
__icmpv6_send(skb_in, type, code, info, &parm);
7880
ipv6_hdr(skb_in)->saddr = orig_ip;
7981
out:

0 commit comments

Comments
 (0)