netdev - Re: [PATCH][unix] missing barriers in some of unix

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] [day] [month] [year] [list]

Date:   Wed, 20 Feb 2019 20:07:08 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     viro@...iv.linux.org.uk
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and
 ->path accesses

From: Al Viro <viro@...iv.linux.org.uk>
Date: Fri, 15 Feb 2019 20:09:35 +0000

> Several u->addr and u->path users are not holding any locks in
> common with unix_bind().  unix_state_lock() is useless for those
> purposes.
> 
> u->addr is assign-once and *(u->addr) is fully set up by the time
> we set u->addr (all under unix_table_lock).  u->path is also
> set in the same critical area, also before setting u->addr, and
> any unix_sock with ->path filled will have non-NULL ->addr.
> 
> So setting ->addr with smp_store_release() is all we need for those
> "lockless" users - just have them fetch ->addr with smp_load_acquire()
> and don't even bother looking at ->path if they see NULL ->addr.
> 
> Users of ->addr and ->path fall into several classes now:
>     1) ones that do smp_load_acquire(u->addr) and access *(u->addr)
> and u->path only if smp_load_acquire() has returned non-NULL.
>     2) places holding unix_table_lock.  These are guaranteed that
> *(u->addr) is seen fully initialized.  If unix_sock is in one of the
> "bound" chains, so's ->path.
>     3) unix_sock_destructor() using ->addr is safe.  All places
> that set u->addr are guaranteed to have seen all stores *(u->addr)
> while holding a reference to u and unix_sock_destructor() is called
> when (atomic) refcount hits zero.
>     4) unix_release_sock() using ->path is safe.  unix_bind()
> is serialized wrt unix_release() (normally - by struct file
> refcount), and for the instances that had ->path set by unix_bind()
> unix_release_sock() comes from unix_release(), so they are fine.
> Instances that had it set in unix_stream_connect() either end up
> attached to a socket (in unix_accept()), in which case the call
> chain to unix_release_sock() and serialization are the same as in
> the previous case, or they never get accept'ed and unix_release_sock()
> is called when the listener is shut down and its queue gets purged.
> In that case the listener's queue lock provides the barriers needed -
> unix_stream_connect() shoves our unix_sock into listener's queue
> under that lock right after having set ->path and eventual
> unix_release_sock() caller picks them from that queue under the
> same lock right before calling unix_release_sock().
>     5) unix_find_other() use of ->path is pointless, but safe -
> it happens with successful lookup by (abstract) name, so ->path.dentry
> is guaranteed to be NULL there.
> 
> earlier-variant-reviewed-by: "Paul E. McKenney" <paulmck@...ux.ibm.com>
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>

Applied and queued up for -stable, thanks Al.