netdev - Re: [PATCH net-next] ipv6: remove RTNL protection from ip6addrlbl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67f5cb70-14a4-4455-8372-f039da2f15c2@kernel.org>
Date: Wed, 3 Apr 2024 10:13:16 -0600
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] ipv6: remove RTNL protection from
 ip6addrlbl_dump()

On 4/3/24 6:39 AM, Eric Dumazet wrote:
> diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
> index 17ac45aa7194ce9c148ed95e14dd575d17feeb98..961e853543b64cd2060ef693ae3ad32a44780aa1 100644
> --- a/net/ipv6/addrlabel.c
> +++ b/net/ipv6/addrlabel.c
> @@ -234,7 +234,8 @@ static int __ip6addrlbl_add(struct net *net, struct ip6addrlbl_entry *newp,
>  		hlist_add_head_rcu(&newp->list, &net->ipv6.ip6addrlbl_table.head);
>  out:
>  	if (!ret)
> -		net->ipv6.ip6addrlbl_table.seq++;
> +		WRITE_ONCE(net->ipv6.ip6addrlbl_table.seq,
> +			   net->ipv6.ip6addrlbl_table.seq + 1);
>  	return ret;
>  }
>  
> @@ -445,7 +446,7 @@ static void ip6addrlbl_putmsg(struct nlmsghdr *nlh,
>  };
>  
>  static int ip6addrlbl_fill(struct sk_buff *skb,
> -			   struct ip6addrlbl_entry *p,
> +			   const struct ip6addrlbl_entry *p,
>  			   u32 lseq,
>  			   u32 portid, u32 seq, int event,
>  			   unsigned int flags)
> @@ -498,7 +499,7 @@ static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  	struct net *net = sock_net(skb->sk);
>  	struct ip6addrlbl_entry *p;
>  	int idx = 0, s_idx = cb->args[0];
> -	int err;
> +	int err = 0;
>  
>  	if (cb->strict_check) {
>  		err = ip6addrlbl_valid_dump_req(nlh, cb->extack);
> @@ -510,7 +511,7 @@ static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  	hlist_for_each_entry_rcu(p, &net->ipv6.ip6addrlbl_table.head, list) {
>  		if (idx >= s_idx) {
>  			err = ip6addrlbl_fill(skb, p,
> -					      net->ipv6.ip6addrlbl_table.seq,
> +					      READ_ONCE(net->ipv6.ip6addrlbl_table.seq),

seems like this should be read once on entry, and the same value used
for all iterations.

>  					      NETLINK_CB(cb->skb).portid,
>  					      nlh->nlmsg_seq,
>  					      RTM_NEWADDRLABEL,
> @@ -522,7 +523,7 @@ static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
>  	}
>  	rcu_read_unlock();
>  	cb->args[0] = idx;
> -	return skb->len;
> +	return err;
>  }
>  
>  static inline int ip6addrlbl_msgsize(void)
> @@ -614,7 +615,7 @@ static int ip6addrlbl_get(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>  
>  	rcu_read_lock();
>  	p = __ipv6_addr_label(net, addr, ipv6_addr_type(addr), ifal->ifal_index);
> -	lseq = net->ipv6.ip6addrlbl_table.seq;
> +	lseq = READ_ONCE(net->ipv6.ip6addrlbl_table.seq);
>  	if (p)
>  		err = ip6addrlbl_fill(skb, p, lseq,
>  				      NETLINK_CB(in_skb).portid,
> @@ -647,6 +648,7 @@ int __init ipv6_addr_label_rtnl_register(void)
>  		return ret;
>  	ret = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETADDRLABEL,
>  				   ip6addrlbl_get,
> -				   ip6addrlbl_dump, RTNL_FLAG_DOIT_UNLOCKED);
> +				   ip6addrlbl_dump, RTNL_FLAG_DOIT_UNLOCKED |
> +						    RTNL_FLAG_DUMP_UNLOCKED);
>  	return ret;
>  }