Skip to content

Commit

Permalink
Merge branch 'net-data-race-annotations'
Browse files Browse the repository at this point in the history
Eric Dumazet says:

====================
net: another round of data-race annotations

Series inspired by some syzbot reports, taking care
of 4 socket fields that can be read locklessly.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
davem330 committed Sep 1, 2023
2 parents 82ba0ff + 251cd40 commit f2e977f
Show file tree
Hide file tree
Showing 16 changed files with 76 additions and 55 deletions.
2 changes: 1 addition & 1 deletion include/net/ip.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static inline void ipcm_init_sk(struct ipcm_cookie *ipcm,
ipcm_init(ipcm);

ipcm->sockc.mark = READ_ONCE(inet->sk.sk_mark);
ipcm->sockc.tsflags = inet->sk.sk_tsflags;
ipcm->sockc.tsflags = READ_ONCE(inet->sk.sk_tsflags);
ipcm->oif = READ_ONCE(inet->sk.sk_bound_dev_if);
ipcm->addr = inet->inet_saddr;
ipcm->protocol = inet->inet_num;
Expand Down
29 changes: 19 additions & 10 deletions include/net/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,12 @@ static inline void sk_wmem_queued_add(struct sock *sk, int val)
WRITE_ONCE(sk->sk_wmem_queued, sk->sk_wmem_queued + val);
}

static inline void sk_forward_alloc_add(struct sock *sk, int val)
{
/* Paired with lockless reads of sk->sk_forward_alloc */
WRITE_ONCE(sk->sk_forward_alloc, sk->sk_forward_alloc + val);
}

void sk_stream_write_space(struct sock *sk);

/* OOB backlog add */
Expand Down Expand Up @@ -1377,7 +1383,7 @@ static inline int sk_forward_alloc_get(const struct sock *sk)
if (sk->sk_prot->forward_alloc_get)
return sk->sk_prot->forward_alloc_get(sk);
#endif
return sk->sk_forward_alloc;
return READ_ONCE(sk->sk_forward_alloc);
}

static inline bool __sk_stream_memory_free(const struct sock *sk, int wake)
Expand Down Expand Up @@ -1673,14 +1679,14 @@ static inline void sk_mem_charge(struct sock *sk, int size)
{
if (!sk_has_account(sk))
return;
sk->sk_forward_alloc -= size;
sk_forward_alloc_add(sk, -size);
}

static inline void sk_mem_uncharge(struct sock *sk, int size)
{
if (!sk_has_account(sk))
return;
sk->sk_forward_alloc += size;
sk_forward_alloc_add(sk, size);
sk_mem_reclaim(sk);
}

Expand Down Expand Up @@ -1900,7 +1906,9 @@ struct sockcm_cookie {
static inline void sockcm_init(struct sockcm_cookie *sockc,
const struct sock *sk)
{
*sockc = (struct sockcm_cookie) { .tsflags = sk->sk_tsflags };
*sockc = (struct sockcm_cookie) {
.tsflags = READ_ONCE(sk->sk_tsflags)
};
}

int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
Expand Down Expand Up @@ -2695,20 +2703,20 @@ void __sock_recv_wifi_status(struct msghdr *msg, struct sock *sk,
static inline void
sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
{
ktime_t kt = skb->tstamp;
struct skb_shared_hwtstamps *hwtstamps = skb_hwtstamps(skb);

u32 tsflags = READ_ONCE(sk->sk_tsflags);
ktime_t kt = skb->tstamp;
/*
* generate control messages if
* - receive time stamping in software requested
* - software time stamp available and wanted
* - hardware time stamps available and wanted
*/
if (sock_flag(sk, SOCK_RCVTSTAMP) ||
(sk->sk_tsflags & SOF_TIMESTAMPING_RX_SOFTWARE) ||
(kt && sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE) ||
(tsflags & SOF_TIMESTAMPING_RX_SOFTWARE) ||
(kt && tsflags & SOF_TIMESTAMPING_SOFTWARE) ||
(hwtstamps->hwtstamp &&
(sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)))
(tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)))
__sock_recv_timestamp(msg, sk, skb);
else
sock_write_timestamp(sk, kt);
Expand All @@ -2730,7 +2738,8 @@ static inline void sock_recv_cmsgs(struct msghdr *msg, struct sock *sk,
#define TSFLAGS_ANY (SOF_TIMESTAMPING_SOFTWARE | \
SOF_TIMESTAMPING_RAW_HARDWARE)

if (sk->sk_flags & FLAGS_RECV_CMSGS || sk->sk_tsflags & TSFLAGS_ANY)
if (sk->sk_flags & FLAGS_RECV_CMSGS ||
READ_ONCE(sk->sk_tsflags) & TSFLAGS_ANY)
__sock_recv_cmsgs(msg, sk, skb);
else if (unlikely(sock_flag(sk, SOCK_TIMESTAMP)))
sock_write_timestamp(sk, skb->tstamp);
Expand Down
10 changes: 6 additions & 4 deletions net/can/j1939/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -974,20 +974,22 @@ static void __j1939_sk_errqueue(struct j1939_session *session, struct sock *sk,
struct sock_exterr_skb *serr;
struct sk_buff *skb;
char *state = "UNK";
u32 tsflags;
int err;

jsk = j1939_sk(sk);

if (!(jsk->state & J1939_SOCK_ERRQUEUE))
return;

tsflags = READ_ONCE(sk->sk_tsflags);
switch (type) {
case J1939_ERRQUEUE_TX_ACK:
if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK))
if (!(tsflags & SOF_TIMESTAMPING_TX_ACK))
return;
break;
case J1939_ERRQUEUE_TX_SCHED:
if (!(sk->sk_tsflags & SOF_TIMESTAMPING_TX_SCHED))
if (!(tsflags & SOF_TIMESTAMPING_TX_SCHED))
return;
break;
case J1939_ERRQUEUE_TX_ABORT:
Expand All @@ -997,7 +999,7 @@ static void __j1939_sk_errqueue(struct j1939_session *session, struct sock *sk,
case J1939_ERRQUEUE_RX_DPO:
fallthrough;
case J1939_ERRQUEUE_RX_ABORT:
if (!(sk->sk_tsflags & SOF_TIMESTAMPING_RX_SOFTWARE))
if (!(tsflags & SOF_TIMESTAMPING_RX_SOFTWARE))
return;
break;
default:
Expand Down Expand Up @@ -1054,7 +1056,7 @@ static void __j1939_sk_errqueue(struct j1939_session *session, struct sock *sk,
}

serr->opt_stats = true;
if (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)
if (tsflags & SOF_TIMESTAMPING_OPT_ID)
serr->ee.ee_data = session->tskey;

netdev_dbg(session->priv->ndev, "%s: 0x%p tskey: %i, state: %s\n",
Expand Down
10 changes: 6 additions & 4 deletions net/core/skbuff.c
Original file line number Diff line number Diff line change
Expand Up @@ -5207,7 +5207,7 @@ static void __skb_complete_tx_timestamp(struct sk_buff *skb,
serr->ee.ee_info = tstype;
serr->opt_stats = opt_stats;
serr->header.h4.iif = skb->dev ? skb->dev->ifindex : 0;
if (sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID) {
if (READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_OPT_ID) {
serr->ee.ee_data = skb_shinfo(skb)->tskey;
if (sk_is_tcp(sk))
serr->ee.ee_data -= atomic_read(&sk->sk_tskey);
Expand Down Expand Up @@ -5263,21 +5263,23 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
{
struct sk_buff *skb;
bool tsonly, opt_stats = false;
u32 tsflags;

if (!sk)
return;

if (!hwtstamps && !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
tsflags = READ_ONCE(sk->sk_tsflags);
if (!hwtstamps && !(tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW) &&
skb_shinfo(orig_skb)->tx_flags & SKBTX_IN_PROGRESS)
return;

tsonly = sk->sk_tsflags & SOF_TIMESTAMPING_OPT_TSONLY;
tsonly = tsflags & SOF_TIMESTAMPING_OPT_TSONLY;
if (!skb_may_tx_timestamp(sk, tsonly))
return;

if (tsonly) {
#ifdef CONFIG_INET
if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
if ((tsflags & SOF_TIMESTAMPING_OPT_STATS) &&
sk_is_tcp(sk)) {
skb = tcp_get_timestamping_opt_stats(sk, orig_skb,
ack_skb);
Expand Down
18 changes: 9 additions & 9 deletions net/core/sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ static int sock_timestamping_bind_phc(struct sock *sk, int phc_index)
if (!match)
return -EINVAL;

sk->sk_bind_phc = phc_index;
WRITE_ONCE(sk->sk_bind_phc, phc_index);

return 0;
}
Expand Down Expand Up @@ -937,7 +937,7 @@ int sock_set_timestamping(struct sock *sk, int optname,
return ret;
}

sk->sk_tsflags = val;
WRITE_ONCE(sk->sk_tsflags, val);
sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW);

if (val & SOF_TIMESTAMPING_RX_SOFTWARE)
Expand Down Expand Up @@ -1045,7 +1045,7 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
mem_cgroup_uncharge_skmem(sk->sk_memcg, pages);
return -ENOMEM;
}
sk->sk_forward_alloc += pages << PAGE_SHIFT;
sk_forward_alloc_add(sk, pages << PAGE_SHIFT);

WRITE_ONCE(sk->sk_reserved_mem,
sk->sk_reserved_mem + (pages << PAGE_SHIFT));
Expand Down Expand Up @@ -1719,8 +1719,8 @@ int sk_getsockopt(struct sock *sk, int level, int optname,

case SO_TIMESTAMPING_OLD:
lv = sizeof(v.timestamping);
v.timestamping.flags = sk->sk_tsflags;
v.timestamping.bind_phc = sk->sk_bind_phc;
v.timestamping.flags = READ_ONCE(sk->sk_tsflags);
v.timestamping.bind_phc = READ_ONCE(sk->sk_bind_phc);
break;

case SO_RCVTIMEO_OLD:
Expand Down Expand Up @@ -3139,10 +3139,10 @@ int __sk_mem_schedule(struct sock *sk, int size, int kind)
{
int ret, amt = sk_mem_pages(size);

sk->sk_forward_alloc += amt << PAGE_SHIFT;
sk_forward_alloc_add(sk, amt << PAGE_SHIFT);
ret = __sk_mem_raise_allocated(sk, size, amt, kind);
if (!ret)
sk->sk_forward_alloc -= amt << PAGE_SHIFT;
sk_forward_alloc_add(sk, -(amt << PAGE_SHIFT));
return ret;
}
EXPORT_SYMBOL(__sk_mem_schedule);
Expand Down Expand Up @@ -3174,7 +3174,7 @@ void __sk_mem_reduce_allocated(struct sock *sk, int amount)
void __sk_mem_reclaim(struct sock *sk, int amount)
{
amount >>= PAGE_SHIFT;
sk->sk_forward_alloc -= amount << PAGE_SHIFT;
sk_forward_alloc_add(sk, -(amount << PAGE_SHIFT));
__sk_mem_reduce_allocated(sk, amount);
}
EXPORT_SYMBOL(__sk_mem_reclaim);
Expand Down Expand Up @@ -3743,7 +3743,7 @@ void sk_get_meminfo(const struct sock *sk, u32 *mem)
mem[SK_MEMINFO_RCVBUF] = READ_ONCE(sk->sk_rcvbuf);
mem[SK_MEMINFO_WMEM_ALLOC] = sk_wmem_alloc_get(sk);
mem[SK_MEMINFO_SNDBUF] = READ_ONCE(sk->sk_sndbuf);
mem[SK_MEMINFO_FWD_ALLOC] = sk->sk_forward_alloc;
mem[SK_MEMINFO_FWD_ALLOC] = sk_forward_alloc_get(sk);
mem[SK_MEMINFO_WMEM_QUEUED] = READ_ONCE(sk->sk_wmem_queued);
mem[SK_MEMINFO_OPTMEM] = atomic_read(&sk->sk_omem_alloc);
mem[SK_MEMINFO_BACKLOG] = READ_ONCE(sk->sk_backlog.len);
Expand Down
2 changes: 1 addition & 1 deletion net/ipv4/ip_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ static int __ip_append_data(struct sock *sk,
paged = !!cork->gso_size;

if (cork->tx_flags & SKBTX_ANY_TSTAMP &&
sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)
READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_OPT_ID)
tskey = atomic_inc_return(&sk->sk_tskey) - 1;

hh_len = LL_RESERVED_SPACE(rt->dst.dev);
Expand Down
2 changes: 1 addition & 1 deletion net/ipv4/ip_sockglue.c
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ static bool ipv4_datagram_support_cmsg(const struct sock *sk,
* or without payload (SOF_TIMESTAMPING_OPT_TSONLY).
*/
info = PKTINFO_SKB_CB(skb);
if (!(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_CMSG) ||
if (!(READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_OPT_CMSG) ||
!info->ipi_ifindex)
return false;

Expand Down
4 changes: 2 additions & 2 deletions net/ipv4/tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2259,14 +2259,14 @@ void tcp_recv_timestamp(struct msghdr *msg, const struct sock *sk,
}
}

if (sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE)
if (READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_SOFTWARE)
has_timestamping = true;
else
tss->ts[0] = (struct timespec64) {0};
}

if (tss->ts[2].tv_sec || tss->ts[2].tv_nsec) {
if (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)
if (READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_RAW_HARDWARE)
has_timestamping = true;
else
tss->ts[2] = (struct timespec64) {0};
Expand Down
2 changes: 1 addition & 1 deletion net/ipv4/tcp_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -3474,7 +3474,7 @@ void sk_forced_mem_schedule(struct sock *sk, int size)
if (delta <= 0)
return;
amt = sk_mem_pages(delta);
sk->sk_forward_alloc += amt << PAGE_SHIFT;
sk_forward_alloc_add(sk, amt << PAGE_SHIFT);
sk_memory_allocated_add(sk, amt);

if (mem_cgroup_sockets_enabled && sk->sk_memcg)
Expand Down
6 changes: 3 additions & 3 deletions net/ipv4/udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1414,9 +1414,9 @@ static void udp_rmem_release(struct sock *sk, int size, int partial,
spin_lock(&sk_queue->lock);


sk->sk_forward_alloc += size;
sk_forward_alloc_add(sk, size);
amt = (sk->sk_forward_alloc - partial) & ~(PAGE_SIZE - 1);
sk->sk_forward_alloc -= amt;
sk_forward_alloc_add(sk, -amt);

if (amt)
__sk_mem_reduce_allocated(sk, amt >> PAGE_SHIFT);
Expand Down Expand Up @@ -1527,7 +1527,7 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
goto uncharge_drop;
}

sk->sk_forward_alloc -= size;
sk_forward_alloc_add(sk, -size);

/* no need to setup a destructor, we will explicitly release the
* forward allocated memory on dequeue
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/ip6_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -1501,7 +1501,7 @@ static int __ip6_append_data(struct sock *sk,
orig_mtu = mtu;

if (cork->tx_flags & SKBTX_ANY_TSTAMP &&
sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)
READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_OPT_ID)
tskey = atomic_inc_return(&sk->sk_tskey) - 1;

hh_len = LL_RESERVED_SPACE(rt->dst.dev);
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/ping.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return -EINVAL;

ipcm6_init_sk(&ipc6, np);
ipc6.sockc.tsflags = sk->sk_tsflags;
ipc6.sockc.tsflags = READ_ONCE(sk->sk_tsflags);
ipc6.sockc.mark = READ_ONCE(sk->sk_mark);

fl6.flowi6_oif = oif;
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/raw.c
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
fl6.flowi6_uid = sk->sk_uid;

ipcm6_init(&ipc6);
ipc6.sockc.tsflags = sk->sk_tsflags;
ipc6.sockc.tsflags = READ_ONCE(sk->sk_tsflags);
ipc6.sockc.mark = fl6.flowi6_mark;

if (sin6) {
Expand Down
2 changes: 1 addition & 1 deletion net/ipv6/udp.c
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)

ipcm6_init(&ipc6);
ipc6.gso_size = READ_ONCE(up->gso_size);
ipc6.sockc.tsflags = sk->sk_tsflags;
ipc6.sockc.tsflags = READ_ONCE(sk->sk_tsflags);
ipc6.sockc.mark = READ_ONCE(sk->sk_mark);

/* destination address check */
Expand Down
Loading

0 comments on commit f2e977f

Please sign in to comment.