netdev - Re: [PATCH V2 net-next] rds: tcp: use rds_destroy_pending() to synchronize netns/module teardown and rds connection/workq management

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, 5 Feb 2018 09:21:00 -0800
From:   Santosh Shilimkar <santosh.shilimkar@...cle.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>,
        netdev@...r.kernel.org
Cc:     davem@...emloft.net
Subject: Re: [PATCH V2 net-next] rds: tcp: use rds_destroy_pending() to
 synchronize netns/module teardown and rds connection/workq management

On 2/3/2018 4:26 AM, Sowmini Varadhan wrote:
> An rds_connection can get added during netns deletion between lines 528
> and 529 of
> 
>    506 static void rds_tcp_kill_sock(struct net *net)
>    :
>    /* code to pull out all the rds_connections that should be destroyed */
>    :
>    528         spin_unlock_irq(&rds_tcp_conn_lock);
>    529         list_for_each_entry_safe(tc, _tc, &tmp_list, t_tcp_node)
>    530                 rds_conn_destroy(tc->t_cpath->cp_conn);
> 
> Such an rds_connection would miss out the rds_conn_destroy()
> loop (that cancels all pending work) and (if it was scheduled
> after netns deletion) could trigger the use-after-free.
> 
> A similar race-window exists for the module unload path
> in rds_tcp_exit -> rds_tcp_destroy_conns
> 
> Concurrency with netns deletion (rds_tcp_kill_sock()) must be handled
> by checking check_net() before enqueuing new work or adding new
> connections.
> 
> Concurrency with module-unload is handled by maintaining a module
> specific flag that is set at the start of the module exit function,
> and must be checked before enqueuing new work or adding new connections.
> 
> This commit refactors existing RDS_DESTROY_PENDING checks added by
> commit 3db6e0d172c9 ("rds: use RCU to synchronize work-enqueue with
> connection teardown") and consolidates all the concurrency checks
> listed above into the function rds_destroy_pending().
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@...cle.com>
> ---
> v2: use check_net() for the netns delete case, as recommended on the list.
>      refactor RDS_DESTROY_PENDING checks and consolidate into
>      rds_destroy_pending()
> 
Thanks for the update. It looks inline as per off-list chat.

Acked-by: Santosh Shilimkar <santosh.shilimkar@...cle.com>