Skip to content

Commit c840ac6

Browse files
committed
crypto: af_alg - Disallow bind/setkey/... after accept(2)
Each af_alg parent socket obtained by socket(2) corresponds to a tfm object once bind(2) has succeeded. An accept(2) call on that parent socket creates a context which then uses the tfm object. Therefore as long as any child sockets created by accept(2) exist the parent socket must not be modified or freed. This patch guarantees this by using locks and a reference count on the parent socket. Any attempt to modify the parent socket will fail with EBUSY. Cc: [email protected] Reported-by: Dmitry Vyukov <[email protected]> Signed-off-by: Herbert Xu <[email protected]>
1 parent dd50458 commit c840ac6

File tree

2 files changed

+35
-8
lines changed

2 files changed

+35
-8
lines changed

crypto/af_alg.c

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,23 @@ int af_alg_release(struct socket *sock)
125125
}
126126
EXPORT_SYMBOL_GPL(af_alg_release);
127127

128+
void af_alg_release_parent(struct sock *sk)
129+
{
130+
struct alg_sock *ask = alg_sk(sk);
131+
bool last;
132+
133+
sk = ask->parent;
134+
ask = alg_sk(sk);
135+
136+
lock_sock(sk);
137+
last = !--ask->refcnt;
138+
release_sock(sk);
139+
140+
if (last)
141+
sock_put(sk);
142+
}
143+
EXPORT_SYMBOL_GPL(af_alg_release_parent);
144+
128145
static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
129146
{
130147
const u32 forbidden = CRYPTO_ALG_INTERNAL;
@@ -133,6 +150,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
133150
struct sockaddr_alg *sa = (void *)uaddr;
134151
const struct af_alg_type *type;
135152
void *private;
153+
int err;
136154

137155
if (sock->state == SS_CONNECTED)
138156
return -EINVAL;
@@ -160,16 +178,22 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
160178
return PTR_ERR(private);
161179
}
162180

181+
err = -EBUSY;
163182
lock_sock(sk);
183+
if (ask->refcnt)
184+
goto unlock;
164185

165186
swap(ask->type, type);
166187
swap(ask->private, private);
167188

189+
err = 0;
190+
191+
unlock:
168192
release_sock(sk);
169193

170194
alg_do_release(type, private);
171195

172-
return 0;
196+
return err;
173197
}
174198

175199
static int alg_setkey(struct sock *sk, char __user *ukey,
@@ -202,11 +226,15 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
202226
struct sock *sk = sock->sk;
203227
struct alg_sock *ask = alg_sk(sk);
204228
const struct af_alg_type *type;
205-
int err = -ENOPROTOOPT;
229+
int err = -EBUSY;
206230

207231
lock_sock(sk);
232+
if (ask->refcnt)
233+
goto unlock;
234+
208235
type = ask->type;
209236

237+
err = -ENOPROTOOPT;
210238
if (level != SOL_ALG || !type)
211239
goto unlock;
212240

@@ -264,7 +292,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)
264292

265293
sk2->sk_family = PF_ALG;
266294

267-
sock_hold(sk);
295+
if (!ask->refcnt++)
296+
sock_hold(sk);
268297
alg_sk(sk2)->parent = sk;
269298
alg_sk(sk2)->type = type;
270299

include/crypto/if_alg.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ struct alg_sock {
3030

3131
struct sock *parent;
3232

33+
unsigned int refcnt;
34+
3335
const struct af_alg_type *type;
3436
void *private;
3537
};
@@ -67,6 +69,7 @@ int af_alg_register_type(const struct af_alg_type *type);
6769
int af_alg_unregister_type(const struct af_alg_type *type);
6870

6971
int af_alg_release(struct socket *sock);
72+
void af_alg_release_parent(struct sock *sk);
7073
int af_alg_accept(struct sock *sk, struct socket *newsock);
7174

7275
int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len);
@@ -83,11 +86,6 @@ static inline struct alg_sock *alg_sk(struct sock *sk)
8386
return (struct alg_sock *)sk;
8487
}
8588

86-
static inline void af_alg_release_parent(struct sock *sk)
87-
{
88-
sock_put(alg_sk(sk)->parent);
89-
}
90-
9189
static inline void af_alg_init_completion(struct af_alg_completion *completion)
9290
{
9391
init_completion(&completion->completion);

0 commit comments

Comments
 (0)