forked from mirrors/linux
		
	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. Cc: <stable@vger.kernel.org> Fixes:e870456d8e("crypto: algif_skcipher - overhaul memory management") Fixes:d887c52d6a("crypto: algif_aead - overhaul memory management") Reported-by: Romain Izard <romain.izard.pro@gmail.com> Signed-off-by: Stephan Mueller <smueller@chronox.de> Tested-by: Romain Izard <romain.izard.pro@gmail.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This commit is contained in:
		
							parent
							
								
									8e1fa89aa8
								
							
						
					
					
						commit
						7d2c3f54e6
					
				
					 4 changed files with 39 additions and 29 deletions
				
			
		|  | @ -1020,6 +1020,18 @@ ssize_t af_alg_sendpage(struct socket *sock, struct page *page, | |||
| } | ||||
| EXPORT_SYMBOL_GPL(af_alg_sendpage); | ||||
| 
 | ||||
| /**
 | ||||
|  * af_alg_free_resources - release resources required for crypto request | ||||
|  */ | ||||
| void af_alg_free_resources(struct af_alg_async_req *areq) | ||||
| { | ||||
| 	struct sock *sk = areq->sk; | ||||
| 
 | ||||
| 	af_alg_free_areq_sgls(areq); | ||||
| 	sock_kfree_s(sk, areq, areq->areqlen); | ||||
| } | ||||
| EXPORT_SYMBOL_GPL(af_alg_free_resources); | ||||
| 
 | ||||
| /**
 | ||||
|  * af_alg_async_cb - AIO callback handler | ||||
|  * | ||||
|  | @ -1036,18 +1048,13 @@ void af_alg_async_cb(struct crypto_async_request *_req, int err) | |||
| 	struct kiocb *iocb = areq->iocb; | ||||
| 	unsigned int resultlen; | ||||
| 
 | ||||
| 	lock_sock(sk); | ||||
| 
 | ||||
| 	/* Buffer size written by crypto operation. */ | ||||
| 	resultlen = areq->outlen; | ||||
| 
 | ||||
| 	af_alg_free_areq_sgls(areq); | ||||
| 	sock_kfree_s(sk, areq, areq->areqlen); | ||||
| 	__sock_put(sk); | ||||
| 	af_alg_free_resources(areq); | ||||
| 	sock_put(sk); | ||||
| 
 | ||||
| 	iocb->ki_complete(iocb, err ? err : resultlen, 0); | ||||
| 
 | ||||
| 	release_sock(sk); | ||||
| } | ||||
| EXPORT_SYMBOL_GPL(af_alg_async_cb); | ||||
| 
 | ||||
|  |  | |||
|  | @ -283,12 +283,23 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg, | |||
| 
 | ||||
| 	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) { | ||||
| 		/* AIO operation */ | ||||
| 		sock_hold(sk); | ||||
| 		areq->iocb = msg->msg_iocb; | ||||
| 		aead_request_set_callback(&areq->cra_u.aead_req, | ||||
| 					  CRYPTO_TFM_REQ_MAY_BACKLOG, | ||||
| 					  af_alg_async_cb, areq); | ||||
| 		err = ctx->enc ? crypto_aead_encrypt(&areq->cra_u.aead_req) : | ||||
| 				 crypto_aead_decrypt(&areq->cra_u.aead_req); | ||||
| 
 | ||||
| 		/* AIO operation in progress */ | ||||
| 		if (err == -EINPROGRESS || err == -EBUSY) { | ||||
| 			/* Remember output size that will be generated. */ | ||||
| 			areq->outlen = outlen; | ||||
| 
 | ||||
| 			return -EIOCBQUEUED; | ||||
| 		} | ||||
| 
 | ||||
| 		sock_put(sk); | ||||
| 	} else { | ||||
| 		/* Synchronous operation */ | ||||
| 		aead_request_set_callback(&areq->cra_u.aead_req, | ||||
|  | @ -300,19 +311,9 @@ static int _aead_recvmsg(struct socket *sock, struct msghdr *msg, | |||
| 				&ctx->wait); | ||||
| 	} | ||||
| 
 | ||||
| 	/* AIO operation in progress */ | ||||
| 	if (err == -EINPROGRESS) { | ||||
| 		sock_hold(sk); | ||||
| 
 | ||||
| 		/* Remember output size that will be generated. */ | ||||
| 		areq->outlen = outlen; | ||||
| 
 | ||||
| 		return -EIOCBQUEUED; | ||||
| 	} | ||||
| 
 | ||||
| free: | ||||
| 	af_alg_free_areq_sgls(areq); | ||||
| 	sock_kfree_s(sk, areq, areq->areqlen); | ||||
| 	af_alg_free_resources(areq); | ||||
| 
 | ||||
| 	return err ? err : outlen; | ||||
| } | ||||
|  |  | |||
|  | @ -117,6 +117,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, | |||
| 
 | ||||
| 	if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) { | ||||
| 		/* AIO operation */ | ||||
| 		sock_hold(sk); | ||||
| 		areq->iocb = msg->msg_iocb; | ||||
| 		skcipher_request_set_callback(&areq->cra_u.skcipher_req, | ||||
| 					      CRYPTO_TFM_REQ_MAY_SLEEP, | ||||
|  | @ -124,6 +125,16 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, | |||
| 		err = ctx->enc ? | ||||
| 			crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) : | ||||
| 			crypto_skcipher_decrypt(&areq->cra_u.skcipher_req); | ||||
| 
 | ||||
| 		/* AIO operation in progress */ | ||||
| 		if (err == -EINPROGRESS || err == -EBUSY) { | ||||
| 			/* Remember output size that will be generated. */ | ||||
| 			areq->outlen = len; | ||||
| 
 | ||||
| 			return -EIOCBQUEUED; | ||||
| 		} | ||||
| 
 | ||||
| 		sock_put(sk); | ||||
| 	} else { | ||||
| 		/* Synchronous operation */ | ||||
| 		skcipher_request_set_callback(&areq->cra_u.skcipher_req, | ||||
|  | @ -136,19 +147,9 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, | |||
| 						 &ctx->wait); | ||||
| 	} | ||||
| 
 | ||||
| 	/* AIO operation in progress */ | ||||
| 	if (err == -EINPROGRESS) { | ||||
| 		sock_hold(sk); | ||||
| 
 | ||||
| 		/* Remember output size that will be generated. */ | ||||
| 		areq->outlen = len; | ||||
| 
 | ||||
| 		return -EIOCBQUEUED; | ||||
| 	} | ||||
| 
 | ||||
| free: | ||||
| 	af_alg_free_areq_sgls(areq); | ||||
| 	sock_kfree_s(sk, areq, areq->areqlen); | ||||
| 	af_alg_free_resources(areq); | ||||
| 
 | ||||
| 	return err ? err : len; | ||||
| } | ||||
|  |  | |||
|  | @ -242,6 +242,7 @@ int af_alg_sendmsg(struct socket *sock, struct msghdr *msg, size_t size, | |||
| 		   unsigned int ivsize); | ||||
| ssize_t af_alg_sendpage(struct socket *sock, struct page *page, | ||||
| 			int offset, size_t size, int flags); | ||||
| void af_alg_free_resources(struct af_alg_async_req *areq); | ||||
| void af_alg_async_cb(struct crypto_async_request *_req, int err); | ||||
| unsigned int af_alg_poll(struct file *file, struct socket *sock, | ||||
| 			 poll_table *wait); | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Stephan Mueller
						Stephan Mueller