sock: Make sock->sk_stamp thread-safe

BugLink: https://bugs.launchpad.net/bugs/1837257
[ Upstream commit 3a0ed3e9619738067214871e9cb826fa23b2ddb9 ]

Al Viro mentioned (Message-ID
<20170626041334.GZ10672@ZenIV.linux.org.uk>)
that there is probably a race condition
lurking in accesses of sk_stamp on 32-bit machines.

sock->sk_stamp is of type ktime_t which is always an s64.
On a 32 bit architecture, we might run into situations of
unsafe access as the access to the field becomes non atomic.

Use seqlocks for synchronization.
This allows us to avoid using spinlocks for readers as
readers do not need mutual exclusion.

Another approach to solve this is to require sk_lock for all
modifications of the timestamps. The current approach allows
for timestamps to have their own lock: sk_stamp_lock.
This allows for the patch to not compete with already
existing critical sections, and side effects are limited
to the paths in the patch.

The addition of the new field maintains the data locality
optimizations from
commit 9115e8cd2a0c ("net: reorganize struct sock for better data
locality")

Note that all the instances of the sk_stamp accesses
are either through the ioctl or the syscall recvmsg.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/include/net/sock.h b/include/net/sock.h
index 5fcdd5b7b47c4eb2bcbabbff9adda0465516ba12..813912810f5e2404b3804f1c0870995838a04d44 100644 (file)
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -293,6 +293,7 @@ struct sock_common {
   *    @sk_filter: socket filtering instructions
   *    @sk_timer: sock cleanup timer
   *    @sk_stamp: time stamp of last packet received
+  *    @sk_stamp_seq: lock for accessing sk_stamp on 32 bit architectures only
   *    @sk_tsflags: SO_TIMESTAMPING socket options
   *    @sk_tskey: counter to disambiguate concurrent tstamp requests
   *    @sk_zckey: counter to order MSG_ZEROCOPY notifications
@@ -462,6 +463,9 @@ struct sock {
        const struct cred       *sk_peer_cred;
        long                    sk_rcvtimeo;
        ktime_t                 sk_stamp;
+#if BITS_PER_LONG==32
+       seqlock_t               sk_stamp_seq;
+#endif
        u16                     sk_tsflags;
        u8                      sk_shutdown;
        u32                     sk_tskey;
@@ -2215,6 +2219,34 @@ static inline void sk_drops_add(struct sock *sk, const struct sk_buff *skb)
        atomic_add(segs, &sk->sk_drops);
 }
 
+static inline ktime_t sock_read_timestamp(struct sock *sk)
+{
+#if BITS_PER_LONG==32
+       unsigned int seq;
+       ktime_t kt;
+
+       do {
+               seq = read_seqbegin(&sk->sk_stamp_seq);
+               kt = sk->sk_stamp;
+       } while (read_seqretry(&sk->sk_stamp_seq, seq));
+
+       return kt;
+#else
+       return sk->sk_stamp;
+#endif
+}
+
+static inline void sock_write_timestamp(struct sock *sk, ktime_t kt)
+{
+#if BITS_PER_LONG==32
+       write_seqlock(&sk->sk_stamp_seq);
+       sk->sk_stamp = kt;
+       write_sequnlock(&sk->sk_stamp_seq);
+#else
+       sk->sk_stamp = kt;
+#endif
+}
+
 void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
                           struct sk_buff *skb);
 void __sock_recv_wifi_status(struct msghdr *msg, struct sock *sk,
@@ -2239,7 +2271,7 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
             (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)))
                __sock_recv_timestamp(msg, sk, skb);
        else
-               sk->sk_stamp = kt;
+               sock_write_timestamp(sk, kt);
 
        if (sock_flag(sk, SOCK_WIFI_STATUS) && skb->wifi_acked_valid)
                __sock_recv_wifi_status(msg, sk, skb);
@@ -2260,9 +2292,9 @@ static inline void sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
        if (sk->sk_flags & FLAGS_TS_OR_DROPS || sk->sk_tsflags & TSFLAGS_ANY)
                __sock_recv_ts_and_drops(msg, sk, skb);
        else if (unlikely(sock_flag(sk, SOCK_TIMESTAMP)))
-               sk->sk_stamp = skb->tstamp;
+               sock_write_timestamp(sk, skb->tstamp);
        else if (unlikely(sk->sk_stamp == SK_DEFAULT_STAMP))
-               sk->sk_stamp = 0;
+               sock_write_timestamp(sk, 0);
 }
 
 void __sock_tx_timestamp(__u16 tsflags, __u8 *tx_flags);

diff --git a/net/compat.c b/net/compat.c
index 32ed993588d64a369fac473563b502cf88a958ff..790851e70dabb2a5420e73525ba0adc228e691f3 100644 (file)
--- a/net/compat.c
+++ b/net/compat.c
@@ -462,12 +462,14 @@ int compat_sock_get_timestamp(struct sock *sk, struct timeval __user *userstamp)
        err = -ENOENT;
        if (!sock_flag(sk, SOCK_TIMESTAMP))
                sock_enable_timestamp(sk, SOCK_TIMESTAMP);
-       tv = ktime_to_timeval(sk->sk_stamp);
+       tv = ktime_to_timeval(sock_read_timestamp(sk));
+
        if (tv.tv_sec == -1)
                return err;
        if (tv.tv_sec == 0) {
-               sk->sk_stamp = ktime_get_real();
-               tv = ktime_to_timeval(sk->sk_stamp);
+               ktime_t kt = ktime_get_real();
+               sock_write_timestamp(sk, kt);
+               tv = ktime_to_timeval(kt);
        }
        err = 0;
        if (put_user(tv.tv_sec, &ctv->tv_sec) ||
@@ -490,12 +492,13 @@ int compat_sock_get_timestampns(struct sock *sk, struct timespec __user *usersta
        err = -ENOENT;
        if (!sock_flag(sk, SOCK_TIMESTAMP))
                sock_enable_timestamp(sk, SOCK_TIMESTAMP);
-       ts = ktime_to_timespec(sk->sk_stamp);
+       ts = ktime_to_timespec(sock_read_timestamp(sk));
        if (ts.tv_sec == -1)
                return err;
        if (ts.tv_sec == 0) {
-               sk->sk_stamp = ktime_get_real();
-               ts = ktime_to_timespec(sk->sk_stamp);
+               ktime_t kt = ktime_get_real();
+               sock_write_timestamp(sk, kt);
+               ts = ktime_to_timespec(kt);
        }
        err = 0;
        if (put_user(ts.tv_sec, &ctv->tv_sec) ||

diff --git a/net/core/sock.c b/net/core/sock.c
index 050dba776f7540d6c40fb11388481bdbe0286357..b7a42fd73b7f885f307bc037ead8425557a03243 100644 (file)
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2732,6 +2732,9 @@ void sock_init_data(struct socket *sock, struct sock *sk)
        sk->sk_sndtimeo         =       MAX_SCHEDULE_TIMEOUT;
 
        sk->sk_stamp = SK_DEFAULT_STAMP;
+#if BITS_PER_LONG==32
+       seqlock_init(&sk->sk_stamp_seq);
+#endif
        atomic_set(&sk->sk_zckey, 0);
 
 #ifdef CONFIG_NET_RX_BUSY_POLL

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index b6543f59d7d50663d80f1e6812b8ef4e917a1ffe..d6771f3b715b690d2f5cfeed69f533e8ccd576a0 100644 (file)
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -585,7 +585,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
                /* Don't enable netstamp, sunrpc doesn't
                   need that much accuracy */
        }
-       svsk->sk_sk->sk_stamp = skb->tstamp;
+       sock_write_timestamp(svsk->sk_sk, skb->tstamp);
        set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags); /* there may be more data... */
 
        len  = skb->len;