Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE

[Santosh Shilimkar <santosh.shilimkar@oracle.com>]

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 <sowmini.varadhan@oracle.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Cc: rds-devel@oss.oracle.com
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>   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 <santosh.shilimkar@oarcle.com>