mirror of
https://github.com/torvalds/linux.git
synced 2024-11-13 23:51:39 +00:00
missing barriers in some of unix_sock ->addr and ->path accesses
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>
This commit is contained in:
parent
a8fef9ba58
commit
ae3b564179
@ -890,7 +890,7 @@ retry:
|
||||
addr->hash ^= sk->sk_type;
|
||||
|
||||
__unix_remove_socket(sk);
|
||||
u->addr = addr;
|
||||
smp_store_release(&u->addr, addr);
|
||||
__unix_insert_socket(&unix_socket_table[addr->hash], sk);
|
||||
spin_unlock(&unix_table_lock);
|
||||
err = 0;
|
||||
@ -1060,7 +1060,7 @@ static int unix_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
|
||||
|
||||
err = 0;
|
||||
__unix_remove_socket(sk);
|
||||
u->addr = addr;
|
||||
smp_store_release(&u->addr, addr);
|
||||
__unix_insert_socket(list, sk);
|
||||
|
||||
out_unlock:
|
||||
@ -1331,15 +1331,29 @@ restart:
|
||||
RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq);
|
||||
otheru = unix_sk(other);
|
||||
|
||||
/* copy address information from listening to new sock*/
|
||||
if (otheru->addr) {
|
||||
refcount_inc(&otheru->addr->refcnt);
|
||||
newu->addr = otheru->addr;
|
||||
}
|
||||
/* copy address information from listening to new sock
|
||||
*
|
||||
* The contents of *(otheru->addr) and otheru->path
|
||||
* 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) {
|
||||
path_get(&otheru->path);
|
||||
newu->path = otheru->path;
|
||||
}
|
||||
refcount_inc(&otheru->addr->refcnt);
|
||||
smp_store_release(&newu->addr, otheru->addr);
|
||||
|
||||
/* Set credentials */
|
||||
copy_peercred(sk, other);
|
||||
@ -1453,7 +1467,7 @@ out:
|
||||
static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
|
||||
{
|
||||
struct sock *sk = sock->sk;
|
||||
struct unix_sock *u;
|
||||
struct unix_address *addr;
|
||||
DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr, uaddr);
|
||||
int err = 0;
|
||||
|
||||
@ -1468,19 +1482,15 @@ static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
|
||||
sock_hold(sk);
|
||||
}
|
||||
|
||||
u = unix_sk(sk);
|
||||
unix_state_lock(sk);
|
||||
if (!u->addr) {
|
||||
addr = smp_load_acquire(&unix_sk(sk)->addr);
|
||||
if (!addr) {
|
||||
sunaddr->sun_family = AF_UNIX;
|
||||
sunaddr->sun_path[0] = 0;
|
||||
err = sizeof(short);
|
||||
} else {
|
||||
struct unix_address *addr = u->addr;
|
||||
|
||||
err = addr->len;
|
||||
memcpy(sunaddr, addr->name, addr->len);
|
||||
}
|
||||
unix_state_unlock(sk);
|
||||
sock_put(sk);
|
||||
out:
|
||||
return err;
|
||||
@ -2073,11 +2083,11 @@ static int unix_seqpacket_recvmsg(struct socket *sock, struct msghdr *msg,
|
||||
|
||||
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) {
|
||||
msg->msg_namelen = u->addr->len;
|
||||
memcpy(msg->msg_name, u->addr->name, u->addr->len);
|
||||
if (addr) {
|
||||
msg->msg_namelen = addr->len;
|
||||
memcpy(msg->msg_name, addr->name, addr->len);
|
||||
}
|
||||
}
|
||||
|
||||
@ -2581,15 +2591,14 @@ static int unix_open_file(struct sock *sk)
|
||||
if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
|
||||
return -EPERM;
|
||||
|
||||
unix_state_lock(sk);
|
||||
path = unix_sk(sk)->path;
|
||||
if (!path.dentry) {
|
||||
unix_state_unlock(sk);
|
||||
if (!smp_load_acquire(&unix_sk(sk)->addr))
|
||||
return -ENOENT;
|
||||
|
||||
path = unix_sk(sk)->path;
|
||||
if (!path.dentry)
|
||||
return -ENOENT;
|
||||
}
|
||||
|
||||
path_get(&path);
|
||||
unix_state_unlock(sk);
|
||||
|
||||
fd = get_unused_fd_flags(O_CLOEXEC);
|
||||
if (fd < 0)
|
||||
@ -2830,7 +2839,7 @@ static int unix_seq_show(struct seq_file *seq, void *v)
|
||||
(s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING),
|
||||
sock_i_ino(s));
|
||||
|
||||
if (u->addr) {
|
||||
if (u->addr) { // under unix_table_lock here
|
||||
int i, len;
|
||||
seq_putc(seq, ' ');
|
||||
|
||||
|
@ -10,7 +10,8 @@
|
||||
|
||||
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)
|
||||
return 0;
|
||||
|
@ -321,6 +321,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
|
||||
if (a->u.net->sk) {
|
||||
struct sock *sk = a->u.net->sk;
|
||||
struct unix_sock *u;
|
||||
struct unix_address *addr;
|
||||
int len = 0;
|
||||
char *p = NULL;
|
||||
|
||||
@ -351,14 +352,15 @@ static void dump_common_audit_data(struct audit_buffer *ab,
|
||||
#endif
|
||||
case AF_UNIX:
|
||||
u = unix_sk(sk);
|
||||
addr = smp_load_acquire(&u->addr);
|
||||
if (!addr)
|
||||
break;
|
||||
if (u->path.dentry) {
|
||||
audit_log_d_path(ab, " path=", &u->path);
|
||||
break;
|
||||
}
|
||||
if (!u->addr)
|
||||
break;
|
||||
len = u->addr->len-sizeof(short);
|
||||
p = &u->addr->name->sun_path[0];
|
||||
len = addr->len-sizeof(short);
|
||||
p = &addr->name->sun_path[0];
|
||||
audit_log_format(ab, " path=");
|
||||
if (*p)
|
||||
audit_log_untrustedstring(ab, p);
|
||||
|
Loading…
Reference in New Issue
Block a user