Re: [PATCH] nvmet-tcp: Fix a possible UAF in queue intialization setup

[sj@kernel.org]

Hello,

On Mon, 2 Oct 2023 13:54:28 +0300 Sagi Grimberg <sagi@grimberg.me> wrote:

> From Alon:
> "Due to a logical bug in the NVMe-oF/TCP subsystem in the Linux kernel,
> a malicious user can cause a UAF and a double free, which may lead to
> RCE (may also lead to an LPE in case the attacker already has local
> privileges)."
> 
> Hence, when a queue initialization fails after the ahash requests are
> allocated, it is guaranteed that the queue removal async work will be
> called, hence leave the deallocation to the queue removal.
> 
> Also, be extra careful not to continue processing the socket, so set
> queue rcv_state to NVMET_TCP_RECV_ERR upon a socket error.
> 
> Reported-by: Alon Zahavi <zahavi.alon@gmail.com>
> Tested-by: Alon Zahavi <zahavi.alon@gmail.com>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>

Would it be better to add Fixes: and Cc: stable lines?


Thanks,
SJ

> ---
>  drivers/nvme/target/tcp.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 97d07488072d..d840f996eb82 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -372,6 +372,7 @@ static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
>  
>  static void nvmet_tcp_socket_error(struct nvmet_tcp_queue *queue, int status)
>  {
> +	queue->rcv_state = NVMET_TCP_RECV_ERR;
>  	if (status == -EPIPE || status == -ECONNRESET)
>  		kernel_sock_shutdown(queue->sock, SHUT_RDWR);
>  	else
> @@ -910,15 +911,11 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
>  	iov.iov_len = sizeof(*icresp);
>  	ret = kernel_sendmsg(queue->sock, &msg, &iov, 1, iov.iov_len);
>  	if (ret < 0)
> -		goto free_crypto;
> +		return ret; /* queue removal will cleanup */
>  
>  	queue->state = NVMET_TCP_Q_LIVE;
>  	nvmet_prepare_receive_pdu(queue);
>  	return 0;
> -free_crypto:
> -	if (queue->hdr_digest || queue->data_digest)
> -		nvmet_tcp_free_crypto(queue);
> -	return ret;
>  }
>  
>  static void nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
> -- 
> 2.41.0
> 
>