netdev - Re: [PATCH net-next v3 5/7] mld: convert ifmcaddr6 to RCU

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]

Date:   Tue, 30 Mar 2021 12:41:40 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org,
        davem@...emloft.net, kuba@...nel.org
Cc:     jwi@...ux.ibm.com, kgraul@...ux.ibm.com, hca@...ux.ibm.com,
        gor@...ux.ibm.com, borntraeger@...ibm.com,
        mareklindner@...mailbox.ch, sw@...onwunderlich.de, a@...table.cc,
        sven@...fation.org, yoshfuji@...ux-ipv6.org, dsahern@...nel.org,
        linux-s390@...r.kernel.org, b.a.t.m.a.n@...ts.open-mesh.org,
        xiyou.wangcong@...il.com
Subject: Re: [PATCH net-next v3 5/7] mld: convert ifmcaddr6 to RCU

On 2021. 3. 30. 오전 4:56, Eric Dumazet wrote:
 >
 >

Hi Eric,
Thank you for the review!

 > On 3/25/21 5:16 PM, Taehee Yoo wrote:
 >> The ifmcaddr6 has been protected by inet6_dev->lock(rwlock) so that
 >> the critical section is atomic context. In order to switch this context,
 >> changing locking is needed. The ifmcaddr6 actually already protected by
 >> RTNL So if it's converted to use RCU, its control path context can be
 >> switched to sleepable.
 >>
 >
 > I do not really understand the changelog.
 >
 > You wanted to convert from RCU to RTNL, right ?

The purpose of this is to use both RCU and RTNL.
In the control path, ifmcaddr6 is protected by RTNL
(setsockopt_needs_rtnl() in the do_ipv6_setsockopt())
And in the data path, ifmcaddr6 will be protected by RCU.

But ifmcaddr6 is already protected by RTNL in the control path.
So, this patch is to convert ifmcaddr6 to RCU only for datapath.
Therefore, by this patch, ifmcaddr6 will be protected by both RTNL and RCU.

I'm so sorry for this strange changelog.

 >
 > Also :
 >
 >> @@ -571,13 +573,9 @@ int ip6_mc_msfget(struct sock *sk, struct 
group_filter *gsf,
 >>   	if (!ipv6_addr_is_multicast(group))
 >>   		return -EINVAL;
 >>
 >> -	rcu_read_lock();
 >> -	idev = ip6_mc_find_dev_rcu(net, group, gsf->gf_interface);
 >> -
 >> -	if (!idev) {
 >> -		rcu_read_unlock();
 >> +	idev = ip6_mc_find_dev_rtnl(net, group, gsf->gf_interface);
 >> +	if (!idev)
 >>   		return -ENODEV;
 >> -	}
 >>
 >
 > I do not see RTNL being acquired before entering ip6_mc_msfget()
 >

Thank you so much for catching this.
I will send a patch to fix this problem!

Thanks a lot!
Taehee Yoo