inet: implement lockless IP_TOS
authorEric Dumazet <edumazet@google.com>
Fri, 22 Sep 2023 03:42:16 +0000 (03:42 +0000)
committerDavid S. Miller <davem@davemloft.net>
Sun, 1 Oct 2023 18:39:18 +0000 (19:39 +0100)
Some reads of inet->tos are racy.

Add needed READ_ONCE() annotations and convert IP_TOS option lockless.

v2: missing changes in include/net/route.h (David Ahern)

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/ip.h
include/net/route.h
net/dccp/ipv4.c
net/ipv4/inet_diag.c
net/ipv4/ip_output.c
net/ipv4/ip_sockglue.c
net/ipv4/tcp_ipv4.c
net/mptcp/sockopt.c
net/sctp/protocol.c
tools/testing/selftests/net/mptcp/mptcp_connect.sh

index 46933a0..6fbc0dc 100644 (file)
@@ -258,7 +258,7 @@ static inline u8 ip_sendmsg_scope(const struct inet_sock *inet,
 
 static inline __u8 get_rttos(struct ipcm_cookie* ipc, struct inet_sock *inet)
 {
-       return (ipc->tos != -1) ? RT_TOS(ipc->tos) : RT_TOS(inet->tos);
+       return (ipc->tos != -1) ? RT_TOS(ipc->tos) : RT_TOS(READ_ONCE(inet->tos));
 }
 
 /* datagram.c */
@@ -810,6 +810,5 @@ int ip_sock_set_mtu_discover(struct sock *sk, int val);
 void ip_sock_set_pktinfo(struct sock *sk);
 void ip_sock_set_recverr(struct sock *sk);
 void ip_sock_set_tos(struct sock *sk, int val);
-void  __ip_sock_set_tos(struct sock *sk, int val);
 
 #endif /* _IP_H */
index 51a45b1..5c248a8 100644 (file)
@@ -37,7 +37,7 @@
 
 #define RTO_ONLINK     0x01
 
-#define RT_CONN_FLAGS(sk)   (RT_TOS(inet_sk(sk)->tos) | sock_flag(sk, SOCK_LOCALROUTE))
+#define RT_CONN_FLAGS(sk)   (RT_TOS(READ_ONCE(inet_sk(sk)->tos)) | sock_flag(sk, SOCK_LOCALROUTE))
 #define RT_CONN_FLAGS_TOS(sk,tos)   (RT_TOS(tos) | sock_flag(sk, SOCK_LOCALROUTE))
 
 static inline __u8 ip_sock_rt_scope(const struct sock *sk)
@@ -50,7 +50,7 @@ static inline __u8 ip_sock_rt_scope(const struct sock *sk)
 
 static inline __u8 ip_sock_rt_tos(const struct sock *sk)
 {
-       return RT_TOS(inet_sk(sk)->tos);
+       return RT_TOS(READ_ONCE(inet_sk(sk)->tos));
 }
 
 struct ip_tunnel_info;
index 69453b9..1b8cbfd 100644 (file)
@@ -511,7 +511,7 @@ static int dccp_v4_send_response(const struct sock *sk, struct request_sock *req
                err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
                                            ireq->ir_rmt_addr,
                                            rcu_dereference(ireq->ireq_opt),
-                                           inet_sk(sk)->tos);
+                                           READ_ONCE(inet_sk(sk)->tos));
                rcu_read_unlock();
                err = net_xmit_eval(err);
        }
index 9f0bd51..f01aee8 100644 (file)
@@ -134,7 +134,7 @@ int inet_diag_msg_attrs_fill(struct sock *sk, struct sk_buff *skb,
         * hence this needs to be included regardless of socket family.
         */
        if (ext & (1 << (INET_DIAG_TOS - 1)))
-               if (nla_put_u8(skb, INET_DIAG_TOS, inet->tos) < 0)
+               if (nla_put_u8(skb, INET_DIAG_TOS, READ_ONCE(inet->tos)) < 0)
                        goto errout;
 
 #if IS_ENABLED(CONFIG_IPV6)
index 9fc7be2..89e62ed 100644 (file)
@@ -544,7 +544,7 @@ EXPORT_SYMBOL(__ip_queue_xmit);
 
 int ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl)
 {
-       return __ip_queue_xmit(sk, skb, fl, inet_sk(sk)->tos);
+       return __ip_queue_xmit(sk, skb, fl, READ_ONCE(inet_sk(sk)->tos));
 }
 EXPORT_SYMBOL(ip_queue_xmit);
 
@@ -1438,7 +1438,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
        iph = ip_hdr(skb);
        iph->version = 4;
        iph->ihl = 5;
-       iph->tos = (cork->tos != -1) ? cork->tos : inet->tos;
+       iph->tos = (cork->tos != -1) ? cork->tos : READ_ONCE(inet->tos);
        iph->frag_off = df;
        iph->ttl = ttl;
        iph->protocol = sk->sk_protocol;
index 6d874cc..50c008e 100644 (file)
@@ -585,25 +585,20 @@ out:
        return err;
 }
 
-void __ip_sock_set_tos(struct sock *sk, int val)
+void ip_sock_set_tos(struct sock *sk, int val)
 {
+       u8 old_tos = READ_ONCE(inet_sk(sk)->tos);
+
        if (sk->sk_type == SOCK_STREAM) {
                val &= ~INET_ECN_MASK;
-               val |= inet_sk(sk)->tos & INET_ECN_MASK;
+               val |= old_tos & INET_ECN_MASK;
        }
-       if (inet_sk(sk)->tos != val) {
-               inet_sk(sk)->tos = val;
+       if (old_tos != val) {
+               WRITE_ONCE(inet_sk(sk)->tos, val);
                WRITE_ONCE(sk->sk_priority, rt_tos2priority(val));
                sk_dst_reset(sk);
        }
 }
-
-void ip_sock_set_tos(struct sock *sk, int val)
-{
-       lock_sock(sk);
-       __ip_sock_set_tos(sk, val);
-       release_sock(sk);
-}
 EXPORT_SYMBOL(ip_sock_set_tos);
 
 void ip_sock_set_freebind(struct sock *sk)
@@ -1050,6 +1045,9 @@ int do_ip_setsockopt(struct sock *sk, int level, int optname,
                return 0;
        case IP_MTU_DISCOVER:
                return ip_sock_set_mtu_discover(sk, val);
+       case IP_TOS:    /* This sets both TOS and Precedence */
+               ip_sock_set_tos(sk, val);
+               return 0;
        }
 
        err = 0;
@@ -1104,9 +1102,6 @@ int do_ip_setsockopt(struct sock *sk, int level, int optname,
                        }
                }
                break;
-       case IP_TOS:    /* This sets both TOS and Precedence */
-               __ip_sock_set_tos(sk, val);
-               break;
        case IP_UNICAST_IF:
        {
                struct net_device *dev = NULL;
@@ -1593,6 +1588,9 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
        case IP_MTU_DISCOVER:
                val = READ_ONCE(inet->pmtudisc);
                goto copyval;
+       case IP_TOS:
+               val = READ_ONCE(inet->tos);
+               goto copyval;
        }
 
        if (needs_rtnl)
@@ -1629,9 +1627,6 @@ int do_ip_getsockopt(struct sock *sk, int level, int optname,
                        return -EFAULT;
                return 0;
        }
-       case IP_TOS:
-               val = inet->tos;
-               break;
        case IP_MTU:
        {
                struct dst_entry *dst;
index 95e972b..a441740 100644 (file)
@@ -1024,10 +1024,11 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
        if (skb) {
                __tcp_v4_send_check(skb, ireq->ir_loc_addr, ireq->ir_rmt_addr);
 
-               tos = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos) ?
-                               (tcp_rsk(req)->syn_tos & ~INET_ECN_MASK) |
-                               (inet_sk(sk)->tos & INET_ECN_MASK) :
-                               inet_sk(sk)->tos;
+               tos = READ_ONCE(inet_sk(sk)->tos);
+
+               if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_reflect_tos))
+                       tos = (tcp_rsk(req)->syn_tos & ~INET_ECN_MASK) |
+                             (tos & INET_ECN_MASK);
 
                if (!INET_ECN_is_capable(tos) &&
                    tcp_bpf_ca_needs_ecn((struct sock *)req))
index f3485a6..18ce624 100644 (file)
@@ -734,11 +734,11 @@ static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
 
        lock_sock(sk);
        sockopt_seq_inc(msk);
-       val = inet_sk(sk)->tos;
+       val = READ_ONCE(inet_sk(sk)->tos);
        mptcp_for_each_subflow(msk, subflow) {
                struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 
-               __ip_sock_set_tos(ssk, val);
+               ip_sock_set_tos(ssk, val);
        }
        release_sock(sk);
 
@@ -1343,7 +1343,7 @@ static int mptcp_getsockopt_v4(struct mptcp_sock *msk, int optname,
 
        switch (optname) {
        case IP_TOS:
-               return mptcp_put_int_option(msk, optval, optlen, inet_sk(sk)->tos);
+               return mptcp_put_int_option(msk, optval, optlen, READ_ONCE(inet_sk(sk)->tos));
        }
 
        return -EOPNOTSUPP;
@@ -1411,7 +1411,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
        ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
        ssk->sk_incoming_cpu = sk->sk_incoming_cpu;
        ssk->sk_ipv6only = sk->sk_ipv6only;
-       __ip_sock_set_tos(ssk, inet_sk(sk)->tos);
+       ip_sock_set_tos(ssk, inet_sk(sk)->tos);
 
        if (sk->sk_userlocks & tx_rx_locks) {
                ssk->sk_userlocks |= sk->sk_userlocks & tx_rx_locks;
index 2185f44..94c6dd5 100644 (file)
@@ -426,7 +426,7 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
        struct dst_entry *dst = NULL;
        union sctp_addr *daddr = &t->ipaddr;
        union sctp_addr dst_saddr;
-       __u8 tos = inet_sk(sk)->tos;
+       u8 tos = READ_ONCE(inet_sk(sk)->tos);
 
        if (t->dscp & SCTP_DSCP_SET_MASK)
                tos = t->dscp & SCTP_DSCP_VAL_MASK;
@@ -1057,7 +1057,7 @@ static inline int sctp_v4_xmit(struct sk_buff *skb, struct sctp_transport *t)
        struct flowi4 *fl4 = &t->fl.u.ip4;
        struct sock *sk = skb->sk;
        struct inet_sock *inet = inet_sk(sk);
-       __u8 dscp = inet->tos;
+       __u8 dscp = READ_ONCE(inet->tos);
        __be16 df = 0;
 
        pr_debug("%s: skb:%p, len:%d, src:%pI4, dst:%pI4\n", __func__, skb,
index b1fc8af..61a2a19 100755 (executable)
@@ -716,7 +716,7 @@ run_test_transparent()
        # the required infrastructure in MPTCP sockopt code. To support TOS, the
        # following function has been exported (T). Not great but better than
        # checking for a specific kernel version.
-       if ! mptcp_lib_kallsyms_has "T __ip_sock_set_tos$"; then
+       if ! mptcp_lib_kallsyms_has "T ip_sock_set_tos$"; then
                echo "INFO: ${msg} not supported by the kernel: SKIP"
                mptcp_lib_result_skip "${TEST_GROUP}"
                return