From mboxrd@z Thu Jan 1 00:00:00 1970 From: Santosh Shilimkar Subject: Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE Date: Mon, 10 Sep 2018 15:43:20 -0700 Message-ID: References: <20180910222422.19470-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Sowmini Varadhan , rds-devel@oss.oracle.com To: Cong Wang , netdev@vger.kernel.org Return-path: Received: from userp2130.oracle.com ([156.151.31.86]:36132 "EHLO userp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726217AbeIKDjp (ORCPT ); Mon, 10 Sep 2018 23:39:45 -0400 In-Reply-To: <20180910222422.19470-1-xiyou.wangcong@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 9/10/2018 3:24 PM, Cong Wang wrote: > When a rds sock is bound, it is inserted into the bind_hash_table > which is protected by RCU. But when releasing rd sock, after it > is removed from this hash table, it is freed immediately without > respecting RCU grace period. This could cause some use-after-free > as reported by syzbot. > Indeed. > Mark the rds sock as SOCK_RCU_FREE before inserting it into the > bind_hash_table, so that it would be always freed after a RCU grace > period. > > Reported-and-tested-by: syzbot+8967084bcac563795dc6@syzkaller.appspotmail.com > Cc: Sowmini Varadhan > Cc: Santosh Shilimkar > Cc: rds-devel@oss.oracle.com > Signed-off-by: Cong Wang > --- > net/rds/bind.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/rds/bind.c b/net/rds/bind.c > index 3ab55784b637..2281b34415b9 100644 > --- a/net/rds/bind.c > +++ b/net/rds/bind.c > @@ -235,6 +235,7 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) > goto out; > } > > + sock_set_flag(sk, SOCK_RCU_FREE); > ret = rds_add_bound(rs, binding_addr, &port, scope_id); > if (ret) > goto out; > I wasn't aware of this "SOCK_RCU_FREE" so really thanks for this patch. Have been scratching my head over this for a while thinking about generic provision at sk level to synchronize. This is much better than adding the sync at upper layer. It does have the tax for slowing down RDS for other kernel components rcu sync but anyway this hole needs to be plugged so am fine to go ahead with this change. Thanks for the patch. FWIW, Acked-by: Santosh Shilimkar