[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ac7f9748-7219-4cf1-961d-df4290ccc689@kernel.org>
Date: Fri, 8 Dec 2023 09:06:43 -0700
From: David Ahern <dsahern@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next 1/2] ipv6: annotate data-races around
np->mcast_oif
On 12/8/23 3:12 AM, Eric Dumazet wrote:
> np->mcast_oif is read locklessly in some contexts.
>
> Make all accesses to this field lockless, adding appropriate
> annotations.
>
> This also makes setsockopt( IPV6_MULTICAST_IF ) lockless.
>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
> net/dccp/ipv6.c | 2 +-
> net/ipv6/datagram.c | 4 +-
> net/ipv6/icmp.c | 4 +-
> net/ipv6/ipv6_sockglue.c | 74 +++++++++++++++++----------------
> net/ipv6/ping.c | 4 +-
> net/ipv6/raw.c | 2 +-
> net/ipv6/tcp_ipv6.c | 2 +-
> net/ipv6/udp.c | 2 +-
> net/l2tp/l2tp_ip6.c | 2 +-
> net/netfilter/ipvs/ip_vs_sync.c | 2 +-
> net/rds/tcp_listen.c | 2 +-
> 11 files changed, 51 insertions(+), 49 deletions(-)
>
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 7d661735cb9d519ab4691979f30365acda0a28c3..fe7e96e69960c013e84b48242e309525f7f618da 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -509,6 +509,34 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> if (optlen < sizeof(int))
> return -EINVAL;
> return ip6_sock_set_addr_preferences(sk, val);
> + case IPV6_MULTICAST_IF:
> + if (sk->sk_type == SOCK_STREAM)
> + return -ENOPROTOOPT;
> + if (optlen < sizeof(int))
> + return -EINVAL;
> + if (val) {
> + struct net_device *dev;
> + int bound_dev_if, midx;
> +
> + rcu_read_lock();
> +
> + dev = dev_get_by_index_rcu(net, val);
> + if (!dev) {
> + rcu_read_unlock();
> + return -ENODEV;
> + }
> + midx = l3mdev_master_ifindex_rcu(dev);
> +
> + rcu_read_unlock();
> +
> + bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
you snuck in an extra change with that code move.
Reviewed-by: David Ahern <dsahern@...nel.org>
> + if (bound_dev_if &&
> + bound_dev_if != val &&
> + (!midx || midx != bound_dev_if))
> + return -EINVAL;
> + }
> + WRITE_ONCE(np->mcast_oif, val);
> + return 0;
> }
> if (needs_rtnl)
> rtnl_lock();
> @@ -860,36 +888,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> break;
> }
>
> - case IPV6_MULTICAST_IF:
> - if (sk->sk_type == SOCK_STREAM)
> - break;
> - if (optlen < sizeof(int))
> - goto e_inval;
> -
> - if (val) {
> - struct net_device *dev;
> - int midx;
> -
> - rcu_read_lock();
> -
> - dev = dev_get_by_index_rcu(net, val);
> - if (!dev) {
> - rcu_read_unlock();
> - retv = -ENODEV;
> - break;
> - }
> - midx = l3mdev_master_ifindex_rcu(dev);
> -
> - rcu_read_unlock();
> -
> - if (sk->sk_bound_dev_if &&
> - sk->sk_bound_dev_if != val &&
> - (!midx || midx != sk->sk_bound_dev_if))
> - goto e_inval;
> - }
> - np->mcast_oif = val;
> - retv = 0;
> - break;
> case IPV6_ADD_MEMBERSHIP:
> case IPV6_DROP_MEMBERSHIP:
> {
Powered by blists - more mailing lists