missing barriers in some of unix_sock ->addr and ->path accesses

[ Upstream commit ae3b564179 ]

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@linux.ibm.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Al Viro 2019-02-15 20:09:35 +00:00 коммит произвёл Greg Kroah-Hartman
Родитель 6c338fafab
Коммит 727a261969
3 изменённых файлов: 42 добавлений и 30 удалений

Просмотреть файл

@ -892,7 +892,7 @@ retry:
addr->hash ^= sk->sk_type; addr->hash ^= sk->sk_type;
__unix_remove_socket(sk); __unix_remove_socket(sk);
u->addr = addr; smp_store_release(&u->addr, addr);
__unix_insert_socket(&unix_socket_table[addr->hash], sk); __unix_insert_socket(&unix_socket_table[addr->hash], sk);
spin_unlock(&unix_table_lock); spin_unlock(&unix_table_lock);
err = 0; err = 0;
@ -1062,7 +1062,7 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
err = 0; err = 0;
__unix_remove_socket(sk); __unix_remove_socket(sk);
u->addr = addr; smp_store_release(&u->addr, addr);
__unix_insert_socket(list, sk); __unix_insert_socket(list, sk);
out_unlock: out_unlock:
@ -1333,15 +1333,29 @@ restart:
RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq); RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq);
otheru = unix_sk(other); otheru = unix_sk(other);
/* copy address information from listening to new sock*/ /* copy address information from listening to new sock
if (otheru->addr) { *
refcount_inc(&otheru->addr->refcnt); * The contents of *(otheru->addr) and otheru->path
newu->addr = otheru->addr; * are seen fully set up here, since we have found
} * otheru in hash under unix_table_lock. Insertion
* into the hash chain we'd found it in had been done
* in an earlier critical area protected by unix_table_lock,
* the same one where we'd set *(otheru->addr) contents,
* as well as otheru->path and otheru->addr itself.
*
* Using smp_store_release() here to set newu->addr
* is enough to make those stores, as well as stores
* to newu->path visible to anyone who gets newu->addr
* by smp_load_acquire(). IOW, the same warranties
* as for unix_sock instances bound in unix_bind() or
* in unix_autobind().
*/
if (otheru->path.dentry) { if (otheru->path.dentry) {
path_get(&otheru->path); path_get(&otheru->path);
newu->path = otheru->path; newu->path = otheru->path;
} }
refcount_inc(&otheru->addr->refcnt);
smp_store_release(&newu->addr, otheru->addr);
/* Set credentials */ /* Set credentials */
copy_peercred(sk, other); copy_peercred(sk, other);
@ -1455,7 +1469,7 @@ out:
static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int *uaddr_len, int peer) static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int *uaddr_len, int peer)
{ {
struct sock *sk = sock->sk; struct sock *sk = sock->sk;
struct unix_sock *u; struct unix_address *addr;
DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr, uaddr); DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr, uaddr);
int err = 0; int err = 0;
@ -1470,19 +1484,15 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int *uaddr_
sock_hold(sk); sock_hold(sk);
} }
u = unix_sk(sk); addr = smp_load_acquire(&unix_sk(sk)->addr);
unix_state_lock(sk); if (!addr) {
if (!u->addr) {
sunaddr->sun_family = AF_UNIX; sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0; sunaddr->sun_path[0] = 0;
*uaddr_len = sizeof(short); *uaddr_len = sizeof(short);
} else { } else {
struct unix_address *addr = u->addr;
*uaddr_len = addr->len; *uaddr_len = addr->len;
memcpy(sunaddr, addr->name, *uaddr_len); memcpy(sunaddr, addr->name, *uaddr_len);
} }
unix_state_unlock(sk);
sock_put(sk); sock_put(sk);
out: out:
return err; return err;
@ -2075,11 +2085,11 @@ static int unix_seqpacket_recvmsg(struct socket *sock, struct msghdr *msg,
static void unix_copy_addr(struct msghdr *msg, struct sock *sk) static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
{ {
struct unix_sock *u = unix_sk(sk); struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr);
if (u->addr) { if (addr) {
msg->msg_namelen = u->addr->len; msg->msg_namelen = addr->len;
memcpy(msg->msg_name, u->addr->name, u->addr->len); memcpy(msg->msg_name, addr->name, addr->len);
} }
} }
@ -2583,15 +2593,14 @@ static int unix_open_file(struct sock *sk)
if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
return -EPERM; return -EPERM;
unix_state_lock(sk); if (!smp_load_acquire(&unix_sk(sk)->addr))
path = unix_sk(sk)->path; return -ENOENT;
if (!path.dentry) {
unix_state_unlock(sk); path = unix_sk(sk)->path;
if (!path.dentry)
return -ENOENT; return -ENOENT;
}
path_get(&path); path_get(&path);
unix_state_unlock(sk);
fd = get_unused_fd_flags(O_CLOEXEC); fd = get_unused_fd_flags(O_CLOEXEC);
if (fd < 0) if (fd < 0)
@ -2831,7 +2840,7 @@ static int unix_seq_show(struct seq_file *seq, void *v)
(s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING), (s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING),
sock_i_ino(s)); sock_i_ino(s));
if (u->addr) { if (u->addr) { // under unix_table_lock here
int i, len; int i, len;
seq_putc(seq, ' '); seq_putc(seq, ' ');

Просмотреть файл

@ -10,7 +10,8 @@
static int sk_diag_dump_name(struct sock *sk, struct sk_buff *nlskb) static int sk_diag_dump_name(struct sock *sk, struct sk_buff *nlskb)
{ {
struct unix_address *addr = unix_sk(sk)->addr; /* might or might not have unix_table_lock */
struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr);
if (!addr) if (!addr)
return 0; return 0;

Просмотреть файл

@ -321,6 +321,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
if (a->u.net->sk) { if (a->u.net->sk) {
struct sock *sk = a->u.net->sk; struct sock *sk = a->u.net->sk;
struct unix_sock *u; struct unix_sock *u;
struct unix_address *addr;
int len = 0; int len = 0;
char *p = NULL; char *p = NULL;
@ -351,14 +352,15 @@ static void dump_common_audit_data(struct audit_buffer *ab,
#endif #endif
case AF_UNIX: case AF_UNIX:
u = unix_sk(sk); u = unix_sk(sk);
addr = smp_load_acquire(&u->addr);
if (!addr)
break;
if (u->path.dentry) { if (u->path.dentry) {
audit_log_d_path(ab, " path=", &u->path); audit_log_d_path(ab, " path=", &u->path);
break; break;
} }
if (!u->addr) len = addr->len-sizeof(short);
break; p = &addr->name->sun_path[0];
len = u->addr->len-sizeof(short);
p = &u->addr->name->sun_path[0];
audit_log_format(ab, " path="); audit_log_format(ab, " path=");
if (*p) if (*p)
audit_log_untrustedstring(ab, p); audit_log_untrustedstring(ab, p);