Skip to content

Commit

Permalink
Merge branch 'ipv6-data-races'
Browse files Browse the repository at this point in the history
Eric Dumazet says:

====================
ipv6: round of data-races fixes

This series is inspired by one related syzbot report.

Many inet6_sk(sk) fields reads or writes are racy.

Move 1-bit fields to inet->inet_flags to provide
atomic safety. inet6_{test|set|clear|assign}_bit() helpers
could be changed later if we need to make room in inet_flags.

Also add missing READ_ONCE()/WRITE_ONCE() when
lockless readers need access to specific fields.

np->srcprefs will be handled separately to avoid merge conflicts
because a prior patch was posted for net tree.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Sep 15, 2023
2 parents f2fa1c8 + 859f8b2 commit e73d5fb
Show file tree
Hide file tree
Showing 24 changed files with 238 additions and 259 deletions.
49 changes: 16 additions & 33 deletions include/linux/ipv6.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,28 +213,9 @@ struct ipv6_pinfo {
__be32 flow_label;
__u32 frag_size;

/*
* Packed in 16bits.
* Omit one shift by putting the signed field at MSB.
*/
#if defined(__BIG_ENDIAN_BITFIELD)
__s16 hop_limit:9;
__u16 __unused_1:7;
#else
__u16 __unused_1:7;
__s16 hop_limit:9;
#endif
s16 hop_limit;
u8 mcast_hops;

#if defined(__BIG_ENDIAN_BITFIELD)
/* Packed in 16bits. */
__s16 mcast_hops:9;
__u16 __unused_2:6,
mc_loop:1;
#else
__u16 mc_loop:1,
__unused_2:6;
__s16 mcast_hops:9;
#endif
int ucast_oif;
int mcast_oif;

Expand Down Expand Up @@ -262,21 +243,11 @@ struct ipv6_pinfo {
} rxopt;

/* sockopt flags */
__u16 recverr:1,
sndflow:1,
repflow:1,
pmtudisc:3,
padding:1, /* 1 bit hole */
srcprefs:3, /* 001: prefer temporary address
__u8 srcprefs:3; /* 001: prefer temporary address
* 010: prefer public address
* 100: prefer care-of address
*/
dontfrag:1,
autoflowlabel:1,
autoflowlabel_set:1,
mc_all:1,
recverr_rfc4884:1,
rtalert_isolate:1;
__u8 pmtudisc;
__u8 min_hopcount;
__u8 tclass;
__be32 rcv_flowinfo;
Expand All @@ -293,6 +264,18 @@ struct ipv6_pinfo {
struct inet6_cork cork;
};

/* We currently use available bits from inet_sk(sk)->inet_flags,
* this could change in the future.
*/
#define inet6_test_bit(nr, sk) \
test_bit(INET_FLAGS_##nr, &inet_sk(sk)->inet_flags)
#define inet6_set_bit(nr, sk) \
set_bit(INET_FLAGS_##nr, &inet_sk(sk)->inet_flags)
#define inet6_clear_bit(nr, sk) \
clear_bit(INET_FLAGS_##nr, &inet_sk(sk)->inet_flags)
#define inet6_assign_bit(nr, sk, val) \
assign_bit(INET_FLAGS_##nr, &inet_sk(sk)->inet_flags, val)

/* WARNING: don't change the layout of the members in {raw,udp,tcp}6_sock! */
struct raw6_sock {
/* inet_sock has to be the first member of raw6_sock */
Expand Down
10 changes: 10 additions & 0 deletions include/net/inet_sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,16 @@ enum {
INET_FLAGS_NODEFRAG = 17,
INET_FLAGS_BIND_ADDRESS_NO_PORT = 18,
INET_FLAGS_DEFER_CONNECT = 19,
INET_FLAGS_MC6_LOOP = 20,
INET_FLAGS_RECVERR6_RFC4884 = 21,
INET_FLAGS_MC6_ALL = 22,
INET_FLAGS_AUTOFLOWLABEL_SET = 23,
INET_FLAGS_AUTOFLOWLABEL = 24,
INET_FLAGS_DONTFRAG = 25,
INET_FLAGS_RECVERR6 = 26,
INET_FLAGS_REPFLOW = 27,
INET_FLAGS_RTALERT_ISOLATE = 28,
INET_FLAGS_SNDFLOW = 29,
};

/* cmsg flags for inet */
Expand Down
14 changes: 9 additions & 5 deletions include/net/ip6_route.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ static inline unsigned int ip6_skb_dst_mtu(const struct sk_buff *skb)
const struct dst_entry *dst = skb_dst(skb);
unsigned int mtu;

if (np && np->pmtudisc >= IPV6_PMTUDISC_PROBE) {
if (np && READ_ONCE(np->pmtudisc) >= IPV6_PMTUDISC_PROBE) {
mtu = READ_ONCE(dst->dev->mtu);
mtu -= lwtunnel_headroom(dst->lwtstate, mtu);
} else {
Expand All @@ -277,14 +277,18 @@ static inline unsigned int ip6_skb_dst_mtu(const struct sk_buff *skb)

static inline bool ip6_sk_accept_pmtu(const struct sock *sk)
{
return inet6_sk(sk)->pmtudisc != IPV6_PMTUDISC_INTERFACE &&
inet6_sk(sk)->pmtudisc != IPV6_PMTUDISC_OMIT;
u8 pmtudisc = READ_ONCE(inet6_sk(sk)->pmtudisc);

return pmtudisc != IPV6_PMTUDISC_INTERFACE &&
pmtudisc != IPV6_PMTUDISC_OMIT;
}

static inline bool ip6_sk_ignore_df(const struct sock *sk)
{
return inet6_sk(sk)->pmtudisc < IPV6_PMTUDISC_DO ||
inet6_sk(sk)->pmtudisc == IPV6_PMTUDISC_OMIT;
u8 pmtudisc = READ_ONCE(inet6_sk(sk)->pmtudisc);

return pmtudisc < IPV6_PMTUDISC_DO ||
pmtudisc == IPV6_PMTUDISC_OMIT;
}

static inline const struct in6_addr *rt6_nexthop(const struct rt6_info *rt,
Expand Down
16 changes: 7 additions & 9 deletions include/net/ipv6.h
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,12 @@ static inline void ipcm6_init(struct ipcm6_cookie *ipc6)
}

static inline void ipcm6_init_sk(struct ipcm6_cookie *ipc6,
const struct ipv6_pinfo *np)
const struct sock *sk)
{
*ipc6 = (struct ipcm6_cookie) {
.hlimit = -1,
.tclass = np->tclass,
.dontfrag = np->dontfrag,
.tclass = inet6_sk(sk)->tclass,
.dontfrag = inet6_test_bit(DONTFRAG, sk),
};
}

Expand Down Expand Up @@ -428,7 +428,7 @@ int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq,
int flags);
int ip6_flowlabel_init(void);
void ip6_flowlabel_cleanup(void);
bool ip6_autoflowlabel(struct net *net, const struct ipv6_pinfo *np);
bool ip6_autoflowlabel(struct net *net, const struct sock *sk);

static inline void fl6_sock_release(struct ip6_flowlabel *fl)
{
Expand Down Expand Up @@ -914,9 +914,9 @@ static inline int ip6_sk_dst_hoplimit(struct ipv6_pinfo *np, struct flowi6 *fl6,
int hlimit;

if (ipv6_addr_is_multicast(&fl6->daddr))
hlimit = np->mcast_hops;
hlimit = READ_ONCE(np->mcast_hops);
else
hlimit = np->hop_limit;
hlimit = READ_ONCE(np->hop_limit);
if (hlimit < 0)
hlimit = ip6_dst_hoplimit(dst);
return hlimit;
Expand Down Expand Up @@ -1303,9 +1303,7 @@ static inline int ip6_sock_set_v6only(struct sock *sk)

static inline void ip6_sock_set_recverr(struct sock *sk)
{
lock_sock(sk);
inet6_sk(sk)->recverr = true;
release_sock(sk);
inet6_set_bit(RECVERR6, sk);
}

static inline int __ip6_sock_set_addr_preferences(struct sock *sk, int val)
Expand Down
2 changes: 1 addition & 1 deletion include/net/sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -2238,7 +2238,7 @@ static inline void sock_confirm_neigh(struct sk_buff *skb, struct neighbour *n)
}
}

bool sk_mc_loop(struct sock *sk);
bool sk_mc_loop(const struct sock *sk);

static inline bool sk_can_gso(const struct sock *sk)
{
Expand Down
2 changes: 1 addition & 1 deletion include/net/xfrm.h
Original file line number Diff line number Diff line change
Expand Up @@ -2166,7 +2166,7 @@ static inline bool xfrm6_local_dontfrag(const struct sock *sk)

proto = sk->sk_protocol;
if (proto == IPPROTO_UDP || proto == IPPROTO_RAW)
return inet6_sk(sk)->dontfrag;
return inet6_test_bit(DONTFRAG, sk);

return false;
}
Expand Down
4 changes: 2 additions & 2 deletions net/core/sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ static int sock_getbindtodevice(struct sock *sk, sockptr_t optval,
return ret;
}

bool sk_mc_loop(struct sock *sk)
bool sk_mc_loop(const struct sock *sk)
{
if (dev_recursion_level())
return false;
Expand All @@ -771,7 +771,7 @@ bool sk_mc_loop(struct sock *sk)
return inet_test_bit(MC_LOOP, sk);
#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6:
return inet6_sk(sk)->mc_loop;
return inet6_test_bit(MC6_LOOP, sk);
#endif
}
WARN_ON_ONCE(1);
Expand Down
8 changes: 4 additions & 4 deletions net/dccp/ipv6.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ static int dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
goto out;
}

if (!sock_owned_by_user(sk) && np->recverr) {
if (!sock_owned_by_user(sk) && inet6_test_bit(RECVERR6, sk)) {
sk->sk_err = err;
sk_error_report(sk);
} else {
Expand Down Expand Up @@ -676,10 +676,10 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
if (np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo)
np->mcast_oif = inet6_iif(opt_skb);
if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
WRITE_ONCE(np->mcast_hops, ipv6_hdr(opt_skb)->hop_limit);
if (np->rxopt.bits.rxflow || np->rxopt.bits.rxtclass)
np->rcv_flowinfo = ip6_flowinfo(ipv6_hdr(opt_skb));
if (np->repflow)
if (inet6_test_bit(REPFLOW, sk))
np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
if (ipv6_opt_accepted(sk, opt_skb,
&DCCP_SKB_CB(opt_skb)->header.h6)) {
Expand Down Expand Up @@ -844,7 +844,7 @@ static int dccp_v6_connect(struct sock *sk, struct sockaddr *uaddr,

memset(&fl6, 0, sizeof(fl6));

if (np->sndflow) {
if (inet6_test_bit(SNDFLOW, sk)) {
fl6.flowlabel = usin->sin6_flowinfo & IPV6_FLOWINFO_MASK;
IP6_ECN_flow_init(fl6.flowlabel);
if (fl6.flowlabel & IPV6_FLOWLABEL_MASK) {
Expand Down
5 changes: 2 additions & 3 deletions net/ipv4/ping.c
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ void ping_err(struct sk_buff *skb, int offset, u32 info)
* 4.1.3.3.
*/
if ((family == AF_INET && !inet_test_bit(RECVERR, sk)) ||
(family == AF_INET6 && !inet6_sk(sk)->recverr)) {
(family == AF_INET6 && !inet6_test_bit(RECVERR6, sk))) {
if (!harderr || sk->sk_state != TCP_ESTABLISHED)
goto out;
} else {
Expand Down Expand Up @@ -899,7 +899,6 @@ int ping_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,

#if IS_ENABLED(CONFIG_IPV6)
} else if (family == AF_INET6) {
struct ipv6_pinfo *np = inet6_sk(sk);
struct ipv6hdr *ip6 = ipv6_hdr(skb);
DECLARE_SOCKADDR(struct sockaddr_in6 *, sin6, msg->msg_name);

Expand All @@ -908,7 +907,7 @@ int ping_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
sin6->sin6_port = 0;
sin6->sin6_addr = ip6->saddr;
sin6->sin6_flowinfo = 0;
if (np->sndflow)
if (inet6_test_bit(SNDFLOW, sk))
sin6->sin6_flowinfo = ip6_flowinfo(ip6);
sin6->sin6_scope_id =
ipv6_iface_scope_id(&sin6->sin6_addr,
Expand Down
9 changes: 5 additions & 4 deletions net/ipv6/af_inet6.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,11 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
inet_sk(sk)->pinet6 = np = inet6_sk_generic(sk);
np->hop_limit = -1;
np->mcast_hops = IPV6_DEFAULT_MCASTHOPS;
np->mc_loop = 1;
np->mc_all = 1;
inet6_set_bit(MC6_LOOP, sk);
inet6_set_bit(MC6_ALL, sk);
np->pmtudisc = IPV6_PMTUDISC_WANT;
np->repflow = net->ipv6.sysctl.flowlabel_reflect & FLOWLABEL_REFLECT_ESTABLISHED;
inet6_assign_bit(REPFLOW, sk, net->ipv6.sysctl.flowlabel_reflect &
FLOWLABEL_REFLECT_ESTABLISHED);
sk->sk_ipv6only = net->ipv6.sysctl.bindv6only;
sk->sk_txrehash = READ_ONCE(net->core.sysctl_txrehash);

Expand Down Expand Up @@ -536,7 +537,7 @@ int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
}
sin->sin6_port = inet->inet_dport;
sin->sin6_addr = sk->sk_v6_daddr;
if (np->sndflow)
if (inet6_test_bit(SNDFLOW, sk))
sin->sin6_flowinfo = np->flow_label;
BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin,
CGROUP_INET6_GETPEERNAME);
Expand Down
15 changes: 7 additions & 8 deletions net/ipv6/datagram.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ int ip6_datagram_dst_update(struct sock *sk, bool fix_sk_saddr)
struct flowi6 fl6;
int err = 0;

if (np->sndflow && (np->flow_label & IPV6_FLOWLABEL_MASK)) {
if (inet6_test_bit(SNDFLOW, sk) &&
(np->flow_label & IPV6_FLOWLABEL_MASK)) {
flowlabel = fl6_sock_lookup(sk, np->flow_label);
if (IS_ERR(flowlabel))
return -EINVAL;
Expand Down Expand Up @@ -163,7 +164,7 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
if (usin->sin6_family != AF_INET6)
return -EAFNOSUPPORT;

if (np->sndflow)
if (inet6_test_bit(SNDFLOW, sk))
fl6_flowlabel = usin->sin6_flowinfo & IPV6_FLOWINFO_MASK;

if (ipv6_addr_any(&usin->sin6_addr)) {
Expand Down Expand Up @@ -305,11 +306,10 @@ static void ipv6_icmp_error_rfc4884(const struct sk_buff *skb,
void ipv6_icmp_error(struct sock *sk, struct sk_buff *skb, int err,
__be16 port, u32 info, u8 *payload)
{
struct ipv6_pinfo *np = inet6_sk(sk);
struct icmp6hdr *icmph = icmp6_hdr(skb);
struct sock_exterr_skb *serr;

if (!np->recverr)
if (!inet6_test_bit(RECVERR6, sk))
return;

skb = skb_clone(skb, GFP_ATOMIC);
Expand All @@ -332,7 +332,7 @@ void ipv6_icmp_error(struct sock *sk, struct sk_buff *skb, int err,

__skb_pull(skb, payload - skb->data);

if (inet6_sk(sk)->recverr_rfc4884)
if (inet6_test_bit(RECVERR6_RFC4884, sk))
ipv6_icmp_error_rfc4884(skb, &serr->ee.ee_rfc4884);

skb_reset_transport_header(skb);
Expand All @@ -344,12 +344,11 @@ EXPORT_SYMBOL_GPL(ipv6_icmp_error);

void ipv6_local_error(struct sock *sk, int err, struct flowi6 *fl6, u32 info)
{
const struct ipv6_pinfo *np = inet6_sk(sk);
struct sock_exterr_skb *serr;
struct ipv6hdr *iph;
struct sk_buff *skb;

if (!np->recverr)
if (!inet6_test_bit(RECVERR6, sk))
return;

skb = alloc_skb(sizeof(struct ipv6hdr), GFP_ATOMIC);
Expand Down Expand Up @@ -493,7 +492,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
const struct ipv6hdr *ip6h = container_of((struct in6_addr *)(nh + serr->addr_offset),
struct ipv6hdr, daddr);
sin->sin6_addr = ip6h->daddr;
if (np->sndflow)
if (inet6_test_bit(SNDFLOW, sk))
sin->sin6_flowinfo = ip6_flowinfo(ip6h);
sin->sin6_scope_id =
ipv6_iface_scope_id(&sin->sin6_addr,
Expand Down
4 changes: 2 additions & 2 deletions net/ipv6/icmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
else if (!fl6.flowi6_oif)
fl6.flowi6_oif = np->ucast_oif;

ipcm6_init_sk(&ipc6, np);
ipcm6_init_sk(&ipc6, sk);
ipc6.sockc.mark = mark;
fl6.flowlabel = ip6_make_flowinfo(ipc6.tclass, fl6.flowlabel);

Expand Down Expand Up @@ -791,7 +791,7 @@ static enum skb_drop_reason icmpv6_echo_reply(struct sk_buff *skb)
msg.offset = 0;
msg.type = type;

ipcm6_init_sk(&ipc6, np);
ipcm6_init_sk(&ipc6, sk);
ipc6.hlimit = ip6_sk_dst_hoplimit(np, &fl6, dst);
ipc6.tclass = ipv6_get_dsfield(ipv6_hdr(skb));
ipc6.sockc.mark = mark;
Expand Down
Loading

0 comments on commit e73d5fb

Please sign in to comment.