Commit cf004b2b authored by Deepa Dinamani's avatar Deepa Dinamani Committed by Juerg Haefliger

sock: Make sock->sk_stamp thread-safe

BugLink: https://bugs.launchpad.net/bugs/1811647

[ Upstream commit 3a0ed3e9 ]

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 9115e8cd ("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: default avatarDeepa Dinamani <deepa.kernel@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarStefan Bader <stefan.bader@canonical.com>
parent 03fd8cf3
...@@ -300,6 +300,7 @@ struct cg_proto; ...@@ -300,6 +300,7 @@ struct cg_proto;
* @sk_filter: socket filtering instructions * @sk_filter: socket filtering instructions
* @sk_timer: sock cleanup timer * @sk_timer: sock cleanup timer
* @sk_stamp: time stamp of last packet received * @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_tsflags: SO_TIMESTAMPING socket options
* @sk_tskey: counter to disambiguate concurrent tstamp requests * @sk_tskey: counter to disambiguate concurrent tstamp requests
* @sk_socket: Identd and reporting IO signals * @sk_socket: Identd and reporting IO signals
...@@ -436,6 +437,9 @@ struct sock { ...@@ -436,6 +437,9 @@ struct sock {
long sk_sndtimeo; long sk_sndtimeo;
struct timer_list sk_timer; struct timer_list sk_timer;
ktime_t sk_stamp; ktime_t sk_stamp;
#if BITS_PER_LONG==32
seqlock_t sk_stamp_seq;
#endif
u16 sk_tsflags; u16 sk_tsflags;
u32 sk_tskey; u32 sk_tskey;
struct socket *sk_socket; struct socket *sk_socket;
...@@ -2171,6 +2175,34 @@ static inline void sk_drops_add(struct sock *sk, const struct sk_buff *skb) ...@@ -2171,6 +2175,34 @@ static inline void sk_drops_add(struct sock *sk, const struct sk_buff *skb)
atomic_add(segs, &sk->sk_drops); 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, void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
struct sk_buff *skb); struct sk_buff *skb);
void __sock_recv_wifi_status(struct msghdr *msg, struct sock *sk, void __sock_recv_wifi_status(struct msghdr *msg, struct sock *sk,
...@@ -2195,7 +2227,7 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb) ...@@ -2195,7 +2227,7 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
(sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE))) (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)))
__sock_recv_timestamp(msg, sk, skb); __sock_recv_timestamp(msg, sk, skb);
else else
sk->sk_stamp = kt; sock_write_timestamp(sk, kt);
if (sock_flag(sk, SOCK_WIFI_STATUS) && skb->wifi_acked_valid) if (sock_flag(sk, SOCK_WIFI_STATUS) && skb->wifi_acked_valid)
__sock_recv_wifi_status(msg, sk, skb); __sock_recv_wifi_status(msg, sk, skb);
...@@ -2215,7 +2247,7 @@ static inline void sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk, ...@@ -2215,7 +2247,7 @@ 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) if (sk->sk_flags & FLAGS_TS_OR_DROPS || sk->sk_tsflags & TSFLAGS_ANY)
__sock_recv_ts_and_drops(msg, sk, skb); __sock_recv_ts_and_drops(msg, sk, skb);
else else
sk->sk_stamp = skb->tstamp; sock_write_timestamp(sk, skb->tstamp);
} }
void __sock_tx_timestamp(const struct sock *sk, __u8 *tx_flags); void __sock_tx_timestamp(const struct sock *sk, __u8 *tx_flags);
......
...@@ -443,12 +443,14 @@ int compat_sock_get_timestamp(struct sock *sk, struct timeval __user *userstamp) ...@@ -443,12 +443,14 @@ int compat_sock_get_timestamp(struct sock *sk, struct timeval __user *userstamp)
err = -ENOENT; err = -ENOENT;
if (!sock_flag(sk, SOCK_TIMESTAMP)) if (!sock_flag(sk, SOCK_TIMESTAMP))
sock_enable_timestamp(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) if (tv.tv_sec == -1)
return err; return err;
if (tv.tv_sec == 0) { if (tv.tv_sec == 0) {
sk->sk_stamp = ktime_get_real(); ktime_t kt = ktime_get_real();
tv = ktime_to_timeval(sk->sk_stamp); sock_write_timestamp(sk, kt);
tv = ktime_to_timeval(kt);
} }
err = 0; err = 0;
if (put_user(tv.tv_sec, &ctv->tv_sec) || if (put_user(tv.tv_sec, &ctv->tv_sec) ||
...@@ -471,12 +473,13 @@ int compat_sock_get_timestampns(struct sock *sk, struct timespec __user *usersta ...@@ -471,12 +473,13 @@ int compat_sock_get_timestampns(struct sock *sk, struct timespec __user *usersta
err = -ENOENT; err = -ENOENT;
if (!sock_flag(sk, SOCK_TIMESTAMP)) if (!sock_flag(sk, SOCK_TIMESTAMP))
sock_enable_timestamp(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) if (ts.tv_sec == -1)
return err; return err;
if (ts.tv_sec == 0) { if (ts.tv_sec == 0) {
sk->sk_stamp = ktime_get_real(); ktime_t kt = ktime_get_real();
ts = ktime_to_timespec(sk->sk_stamp); sock_write_timestamp(sk, kt);
ts = ktime_to_timespec(kt);
} }
err = 0; err = 0;
if (put_user(ts.tv_sec, &ctv->tv_sec) || if (put_user(ts.tv_sec, &ctv->tv_sec) ||
......
...@@ -2425,6 +2425,9 @@ void sock_init_data(struct socket *sock, struct sock *sk) ...@@ -2425,6 +2425,9 @@ void sock_init_data(struct socket *sock, struct sock *sk)
sk->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT; sk->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT;
sk->sk_stamp = ktime_set(-1L, 0); sk->sk_stamp = ktime_set(-1L, 0);
#if BITS_PER_LONG==32
seqlock_init(&sk->sk_stamp_seq);
#endif
#ifdef CONFIG_NET_RX_BUSY_POLL #ifdef CONFIG_NET_RX_BUSY_POLL
sk->sk_napi_id = 0; sk->sk_napi_id = 0;
......
...@@ -614,7 +614,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp) ...@@ -614,7 +614,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
/* Don't enable netstamp, sunrpc doesn't /* Don't enable netstamp, sunrpc doesn't
need that much accuracy */ 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... */ set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags); /* there may be more data... */
len = skb->len - sizeof(struct udphdr); len = skb->len - sizeof(struct udphdr);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment