netdev - Re: [Patch net] rds: mark bound socket with SOCK_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:   Mon, 10 Sep 2018 20:04:37 -0400
From:   Sowmini Varadhan <sowmini.varadhan@...cle.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Santosh Shilimkar <santosh.shilimkar@...cle.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        rds-devel@....oracle.com
Subject: Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE

On (09/10/18 16:51), Cong Wang wrote:
> 
>         __rds_create_bind_key(key, addr, port, scope_id);
> -       rs = rhashtable_lookup_fast(&bind_hash_table, key, ht_parms);
> +       rcu_read_lock();
> +       rs = rhashtable_lookup(&bind_hash_table, key, ht_parms);
>         if (rs && !sock_flag(rds_rs_to_sk(rs), SOCK_DEAD))
>                 rds_sock_addref(rs);
>         else
>                 rs = NULL;
> +       rcu_read_unlock();

aiui, the rcu_read lock/unlock is only useful if the write
side doing destructive operations  does something to make sure readers
are done before doing the destructive opertion. AFAIK, that does
not exist for rds socket management today

> Although sock release path is not a very hot path, but blocking
> it isn't a good idea either, especially when you can use call_rcu(),
> which has the same effect.
> 
> I don't see any reason we should prefer synchronize_rcu() here.

Usually correctness (making sure all readers are done, before nuking a
data structure) is a little bit more important than perforamance, aka
"safety before speed" is what I've always been taught.  

Clearly, your mileage varies. As you please.

--Sowmini