crypto: af_alg - remove locking in async callback

The code paths protected by the socket-lock do not use or modify the socket in a non-atomic fashion. The actions pertaining the socket do not even need to be handled as an atomic operation. Thus, the socket-lock can be safely ignored. This fixes a bug regarding scheduling in atomic as the callback function may be invoked in interrupt context. In addition, the sock_hold is moved before the AIO encrypt/decrypt operation to ensure that the socket is always present. This avoids a tiny race window where the socket is unprotected and yet used by the AIO operation. Finally, the release of resources for a crypto operation is moved into a common function of af_alg_free_resources.

Stephan Mueller committed 9 years ago

Commit 7d2c3f51 parent 8e1fa894 files · +39 −29Browse files

Unified · Split

4 files

crypto/af_alg.c +14 −7
crypto/algif_aead.c +12 −11
crypto/algif_skcipher.c +12 −11
include/crypto/if_alg.h +1 −0

crypto/af_alg.c+14−7

@@ -1020,6 +1020,18 @@ unlock: · 1019 unmodified lines
1020	1020	}
1021	1021	EXPORT_SYMBOL_GPL(af_alg_sendpage);
1022	1022
	1023	+/**
	1024	+ * af_alg_free_resources - release resources required for crypto request
	1025	+ */
	1026	+void af_alg_free_resources(struct af_alg_async_req *areq)
	1027	+{
	1028	+ struct sock *sk = areq->sk;
	1029	+
	1030	+ af_alg_free_areq_sgls(areq);
	1031	+ sock_kfree_s(sk, areq, areq->areqlen);
	1032	+}
	1033	+EXPORT_SYMBOL_GPL(af_alg_free_resources);
	1034	+
1023	1035	/**
1024	1036	* af_alg_async_cb - AIO callback handler
1025	1037	*
@@ -1036,18 +1048,13 @@ void af_alg_async_cb(struct crypto_async_request *_req, int err) · 10 unmodified lines
1036	1048	struct kiocb *iocb = areq->iocb;
1037	1049	unsigned int resultlen;
1038	1050
1039		− lock_sock(sk);
1040		−
1041	1051	/* Buffer size written by crypto operation. */
1042	1052	resultlen = areq->outlen;
1043	1053
1044		− af_alg_free_areq_sgls(areq);
1045		− sock_kfree_s(sk, areq, areq->areqlen);
1046		− __sock_put(sk);
	1054	+ af_alg_free_resources(areq);
	1055	+ sock_put(sk);
1047	1056
1048	1057	iocb->ki_complete(iocb, err ? err : resultlen, 0);
1049		−
1050		− release_sock(sk);
1051	1058	}
1052	1059	EXPORT_SYMBOL_GPL(af_alg_async_cb);
1053	1060

crypto/algif_aead.c+12−11

@@ -283,12 +283,23 @@ static int _aead_recvmsg(struct socket sock, struct msghdr msg, · 282 unmodified lines
283	283
284	284	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
285	285	/* AIO operation */
	286	+ sock_hold(sk);
286	287	areq->iocb = msg->msg_iocb;
287	288	aead_request_set_callback(&areq->cra_u.aead_req,
288	289	CRYPTO_TFM_REQ_MAY_BACKLOG,
289	290	af_alg_async_cb, areq);
290	291	err = ctx->enc ? crypto_aead_encrypt(&areq->cra_u.aead_req) :
291	292	crypto_aead_decrypt(&areq->cra_u.aead_req);
	293	+
	294	+ /* AIO operation in progress */
	295	+ if (err == -EINPROGRESS \|\| err == -EBUSY) {
	296	+ /* Remember output size that will be generated. */
	297	+ areq->outlen = outlen;
	298	+
	299	+ return -EIOCBQUEUED;
	300	+ }
	301	+
	302	+ sock_put(sk);
292	303	} else {
293	304	/* Synchronous operation */
294	305	aead_request_set_callback(&areq->cra_u.aead_req,
@@ -300,19 +311,9 @@ static int _aead_recvmsg(struct socket sock, struct msghdr msg, · 5 unmodified lines
300	311	&ctx->wait);
301	312	}
302	313
303		− /* AIO operation in progress */
304		− if (err == -EINPROGRESS) {
305		− sock_hold(sk);
306		−
307		− /* Remember output size that will be generated. */
308		− areq->outlen = outlen;
309		−
310		− return -EIOCBQUEUED;
311		− }
312	314
313	315	free:
314		− af_alg_free_areq_sgls(areq);
315		− sock_kfree_s(sk, areq, areq->areqlen);
	316	+ af_alg_free_resources(areq);
316	317
317	318	return err ? err : outlen;
318	319	}

crypto/algif_skcipher.c+12−11

@@ -117,6 +117,7 @@ static int _skcipher_recvmsg(struct socket sock, struct msghdr msg, · 116 unmodified lines
117	117
118	118	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
119	119	/* AIO operation */
	120	+ sock_hold(sk);
120	121	areq->iocb = msg->msg_iocb;
121	122	skcipher_request_set_callback(&areq->cra_u.skcipher_req,
122	123	CRYPTO_TFM_REQ_MAY_SLEEP,
@@ -124,6 +125,16 @@ static int _skcipher_recvmsg(struct socket sock, struct msghdr msg, · 1 unmodified line
124	125	err = ctx->enc ?
125	126	crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) :
126	127	crypto_skcipher_decrypt(&areq->cra_u.skcipher_req);
	128	+
	129	+ /* AIO operation in progress */
	130	+ if (err == -EINPROGRESS \|\| err == -EBUSY) {
	131	+ /* Remember output size that will be generated. */
	132	+ areq->outlen = len;
	133	+
	134	+ return -EIOCBQUEUED;
	135	+ }
	136	+
	137	+ sock_put(sk);
127	138	} else {
128	139	/* Synchronous operation */
129	140	skcipher_request_set_callback(&areq->cra_u.skcipher_req,
@@ -136,19 +147,9 @@ static int _skcipher_recvmsg(struct socket sock, struct msghdr msg, · 6 unmodified lines
136	147	&ctx->wait);
137	148	}
138	149
139		− /* AIO operation in progress */
140		− if (err == -EINPROGRESS) {
141		− sock_hold(sk);
142		−
143		− /* Remember output size that will be generated. */
144		− areq->outlen = len;
145		−
146		− return -EIOCBQUEUED;
147		− }
148	150
149	151	free:
150		− af_alg_free_areq_sgls(areq);
151		− sock_kfree_s(sk, areq, areq->areqlen);
	152	+ af_alg_free_resources(areq);
152	153
153	154	return err ? err : len;
154	155	}

include/crypto/if_alg.h+1−0

@@ -242,6 +242,7 @@ int af_alg_sendmsg(struct socket sock, struct msghdr msg, size_t size, · 241 unmodified lines
242	242	unsigned int ivsize);
243	243	ssize_t af_alg_sendpage(struct socket sock, struct page page,
244	244	int offset, size_t size, int flags);
	245	+void af_alg_free_resources(struct af_alg_async_req *areq);
245	246	void af_alg_async_cb(struct crypto_async_request *_req, int err);
246	247	unsigned int af_alg_poll(struct file file, struct socket sock,
247	248	poll_table *wait);