]>
Commit | Line | Data |
---|---|---|
8cbcfc8d GKH |
1 | From foo@baz Fri Mar 15 20:48:31 PDT 2019 |
2 | From: Al Viro <viro@zeniv.linux.org.uk> | |
3 | Date: Fri, 15 Feb 2019 20:09:35 +0000 | |
4 | Subject: missing barriers in some of unix_sock ->addr and ->path accesses | |
5 | ||
6 | From: Al Viro <viro@zeniv.linux.org.uk> | |
7 | ||
8 | [ Upstream commit ae3b564179bfd06f32d051b9e5d72ce4b2a07c37 ] | |
9 | ||
10 | Several u->addr and u->path users are not holding any locks in | |
11 | common with unix_bind(). unix_state_lock() is useless for those | |
12 | purposes. | |
13 | ||
14 | u->addr is assign-once and *(u->addr) is fully set up by the time | |
15 | we set u->addr (all under unix_table_lock). u->path is also | |
16 | set in the same critical area, also before setting u->addr, and | |
17 | any unix_sock with ->path filled will have non-NULL ->addr. | |
18 | ||
19 | So setting ->addr with smp_store_release() is all we need for those | |
20 | "lockless" users - just have them fetch ->addr with smp_load_acquire() | |
21 | and don't even bother looking at ->path if they see NULL ->addr. | |
22 | ||
23 | Users of ->addr and ->path fall into several classes now: | |
24 | 1) ones that do smp_load_acquire(u->addr) and access *(u->addr) | |
25 | and u->path only if smp_load_acquire() has returned non-NULL. | |
26 | 2) places holding unix_table_lock. These are guaranteed that | |
27 | *(u->addr) is seen fully initialized. If unix_sock is in one of the | |
28 | "bound" chains, so's ->path. | |
29 | 3) unix_sock_destructor() using ->addr is safe. All places | |
30 | that set u->addr are guaranteed to have seen all stores *(u->addr) | |
31 | while holding a reference to u and unix_sock_destructor() is called | |
32 | when (atomic) refcount hits zero. | |
33 | 4) unix_release_sock() using ->path is safe. unix_bind() | |
34 | is serialized wrt unix_release() (normally - by struct file | |
35 | refcount), and for the instances that had ->path set by unix_bind() | |
36 | unix_release_sock() comes from unix_release(), so they are fine. | |
37 | Instances that had it set in unix_stream_connect() either end up | |
38 | attached to a socket (in unix_accept()), in which case the call | |
39 | chain to unix_release_sock() and serialization are the same as in | |
40 | the previous case, or they never get accept'ed and unix_release_sock() | |
41 | is called when the listener is shut down and its queue gets purged. | |
42 | In that case the listener's queue lock provides the barriers needed - | |
43 | unix_stream_connect() shoves our unix_sock into listener's queue | |
44 | under that lock right after having set ->path and eventual | |
45 | unix_release_sock() caller picks them from that queue under the | |
46 | same lock right before calling unix_release_sock(). | |
47 | 5) unix_find_other() use of ->path is pointless, but safe - | |
48 | it happens with successful lookup by (abstract) name, so ->path.dentry | |
49 | is guaranteed to be NULL there. | |
50 | ||
51 | earlier-variant-reviewed-by: "Paul E. McKenney" <paulmck@linux.ibm.com> | |
52 | Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> | |
53 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
54 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
55 | --- | |
56 | net/unix/af_unix.c | 48 +++++++++++++++++++++++++++++------------------- | |
57 | net/unix/diag.c | 3 ++- | |
58 | security/lsm_audit.c | 10 ++++++---- | |
59 | 3 files changed, 37 insertions(+), 24 deletions(-) | |
60 | ||
61 | --- a/net/unix/af_unix.c | |
62 | +++ b/net/unix/af_unix.c | |
63 | @@ -891,7 +891,7 @@ retry: | |
64 | addr->hash ^= sk->sk_type; | |
65 | ||
66 | __unix_remove_socket(sk); | |
67 | - u->addr = addr; | |
68 | + smp_store_release(&u->addr, addr); | |
69 | __unix_insert_socket(&unix_socket_table[addr->hash], sk); | |
70 | spin_unlock(&unix_table_lock); | |
71 | err = 0; | |
72 | @@ -1061,7 +1061,7 @@ static int unix_bind(struct socket *sock | |
73 | ||
74 | err = 0; | |
75 | __unix_remove_socket(sk); | |
76 | - u->addr = addr; | |
77 | + smp_store_release(&u->addr, addr); | |
78 | __unix_insert_socket(list, sk); | |
79 | ||
80 | out_unlock: | |
81 | @@ -1332,15 +1332,29 @@ restart: | |
82 | RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq); | |
83 | otheru = unix_sk(other); | |
84 | ||
85 | - /* copy address information from listening to new sock*/ | |
86 | - if (otheru->addr) { | |
87 | - atomic_inc(&otheru->addr->refcnt); | |
88 | - newu->addr = otheru->addr; | |
89 | - } | |
90 | + /* copy address information from listening to new sock | |
91 | + * | |
92 | + * The contents of *(otheru->addr) and otheru->path | |
93 | + * are seen fully set up here, since we have found | |
94 | + * otheru in hash under unix_table_lock. Insertion | |
95 | + * into the hash chain we'd found it in had been done | |
96 | + * in an earlier critical area protected by unix_table_lock, | |
97 | + * the same one where we'd set *(otheru->addr) contents, | |
98 | + * as well as otheru->path and otheru->addr itself. | |
99 | + * | |
100 | + * Using smp_store_release() here to set newu->addr | |
101 | + * is enough to make those stores, as well as stores | |
102 | + * to newu->path visible to anyone who gets newu->addr | |
103 | + * by smp_load_acquire(). IOW, the same warranties | |
104 | + * as for unix_sock instances bound in unix_bind() or | |
105 | + * in unix_autobind(). | |
106 | + */ | |
107 | if (otheru->path.dentry) { | |
108 | path_get(&otheru->path); | |
109 | newu->path = otheru->path; | |
110 | } | |
111 | + atomic_inc(&otheru->addr->refcnt); | |
112 | + smp_store_release(&newu->addr, otheru->addr); | |
113 | ||
114 | /* Set credentials */ | |
115 | copy_peercred(sk, other); | |
116 | @@ -1453,7 +1467,7 @@ out: | |
117 | static int unix_getname(struct socket *sock, struct sockaddr *uaddr, int *uaddr_len, int peer) | |
118 | { | |
119 | struct sock *sk = sock->sk; | |
120 | - struct unix_sock *u; | |
121 | + struct unix_address *addr; | |
122 | DECLARE_SOCKADDR(struct sockaddr_un *, sunaddr, uaddr); | |
123 | int err = 0; | |
124 | ||
125 | @@ -1468,19 +1482,15 @@ static int unix_getname(struct socket *s | |
126 | sock_hold(sk); | |
127 | } | |
128 | ||
129 | - u = unix_sk(sk); | |
130 | - unix_state_lock(sk); | |
131 | - if (!u->addr) { | |
132 | + addr = smp_load_acquire(&unix_sk(sk)->addr); | |
133 | + if (!addr) { | |
134 | sunaddr->sun_family = AF_UNIX; | |
135 | sunaddr->sun_path[0] = 0; | |
136 | *uaddr_len = sizeof(short); | |
137 | } else { | |
138 | - struct unix_address *addr = u->addr; | |
139 | - | |
140 | *uaddr_len = addr->len; | |
141 | memcpy(sunaddr, addr->name, *uaddr_len); | |
142 | } | |
143 | - unix_state_unlock(sk); | |
144 | sock_put(sk); | |
145 | out: | |
146 | return err; | |
147 | @@ -2094,11 +2104,11 @@ static int unix_seqpacket_recvmsg(struct | |
148 | ||
149 | static void unix_copy_addr(struct msghdr *msg, struct sock *sk) | |
150 | { | |
151 | - struct unix_sock *u = unix_sk(sk); | |
152 | + struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr); | |
153 | ||
154 | - if (u->addr) { | |
155 | - msg->msg_namelen = u->addr->len; | |
156 | - memcpy(msg->msg_name, u->addr->name, u->addr->len); | |
157 | + if (addr) { | |
158 | + msg->msg_namelen = addr->len; | |
159 | + memcpy(msg->msg_name, addr->name, addr->len); | |
160 | } | |
161 | } | |
162 | ||
163 | @@ -2814,7 +2824,7 @@ static int unix_seq_show(struct seq_file | |
164 | (s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING), | |
165 | sock_i_ino(s)); | |
166 | ||
167 | - if (u->addr) { | |
168 | + if (u->addr) { // under unix_table_lock here | |
169 | int i, len; | |
170 | seq_putc(seq, ' '); | |
171 | ||
172 | --- a/net/unix/diag.c | |
173 | +++ b/net/unix/diag.c | |
174 | @@ -10,7 +10,8 @@ | |
175 | ||
176 | static int sk_diag_dump_name(struct sock *sk, struct sk_buff *nlskb) | |
177 | { | |
178 | - struct unix_address *addr = unix_sk(sk)->addr; | |
179 | + /* might or might not have unix_table_lock */ | |
180 | + struct unix_address *addr = smp_load_acquire(&unix_sk(sk)->addr); | |
181 | ||
182 | if (!addr) | |
183 | return 0; | |
184 | --- a/security/lsm_audit.c | |
185 | +++ b/security/lsm_audit.c | |
186 | @@ -321,6 +321,7 @@ static void dump_common_audit_data(struc | |
187 | if (a->u.net->sk) { | |
188 | struct sock *sk = a->u.net->sk; | |
189 | struct unix_sock *u; | |
190 | + struct unix_address *addr; | |
191 | int len = 0; | |
192 | char *p = NULL; | |
193 | ||
194 | @@ -351,14 +352,15 @@ static void dump_common_audit_data(struc | |
195 | #endif | |
196 | case AF_UNIX: | |
197 | u = unix_sk(sk); | |
198 | + addr = smp_load_acquire(&u->addr); | |
199 | + if (!addr) | |
200 | + break; | |
201 | if (u->path.dentry) { | |
202 | audit_log_d_path(ab, " path=", &u->path); | |
203 | break; | |
204 | } | |
205 | - if (!u->addr) | |
206 | - break; | |
207 | - len = u->addr->len-sizeof(short); | |
208 | - p = &u->addr->name->sun_path[0]; | |
209 | + len = addr->len-sizeof(short); | |
210 | + p = &addr->name->sun_path[0]; | |
211 | audit_log_format(ab, " path="); | |
212 | if (*p) | |
213 | audit_log_untrustedstring(ab, p); |