linux-kernel - Re: Re: nvme-tcp: fix a possible UAF when failing to send request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202502121747455267343@cestc.cn>
Date: Wed, 12 Feb 2025 17:47:45 +0800
From: "zhang.guanghui@...tc.cn" <zhang.guanghui@...tc.cn>
To: "Maurizio Lombardi" <mlombard@...backstore.eu>, 
	chunguang.xu <chunguang.xu@...pee.com>
Cc: mgurtovoy <mgurtovoy@...dia.com>, 
	sagi <sagi@...mberg.me>, 
	kbusch <kbusch@...nel.org>, 
	sashal <sashal@...nel.org>, 
	linux-kernel <linux-kernel@...r.kernel.org>, 
	linux-nvme <linux-nvme@...ts.infradead.org>, 
	linux-block <linux-block@...r.kernel.org>
Subject: Re: Re: nvme-tcp: fix a possible UAF when failing to send request

    Hi, Thanks.
    I will test this patch, but I am worried whether it will affect the performance.
Should we also consider null pointer protection?


zhang.guanghui@...tc.cn



From: Maurizio Lombardi



Date: 2025-02-12 16:52



To: Maurizio Lombardi; zhang.guanghui@...tc.cn; chunguang.xu



CC: mgurtovoy; sagi; kbusch; sashal; linux-kernel; linux-nvme; linux-block



Subject: Re: nvme-tcp: fix a possible UAF when failing to send request



On Wed Feb 12, 2025 at 9:11 AM CET, Maurizio Lombardi wrote:



> On Tue Feb 11, 2025 at 9:04 AM CET, zhang.guanghui@...tc.cn wrote:



>> Hi 



>>



>>     This is a  race issue,  I can't reproduce it stably yet. I have not tested the latest kernel.  but in fact,  I've synced some nvme-tcp patches from  lastest upstream,



>



> Hello, could you try this patch?



>



> queue_lock should protect against concurrent "error recovery",



> + mutex_lock(&queue->queue_lock);



 



Unfortunately I've just realized that queue_lock won't save us



from the race against the controller reset, it's still possible



we lock a destroyed mutex. So just try this



simplified patch, I will try to figure out something else:



 



diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c



index 841238f38fdd..b714e1691c30 100644



--- a/drivers/nvme/host/tcp.c



+++ b/drivers/nvme/host/tcp.c



@@ -2660,7 +2660,10 @@ static int nvme_tcp_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)



set_bit(NVME_TCP_Q_POLLING, &queue->flags);



if (sk_can_busy_loop(sk) && skb_queue_empty_lockless(&sk->sk_receive_queue))



sk_busy_loop(sk, true);



+



+ mutex_lock(&queue->send_mutex);



nvme_tcp_try_recv(queue);



+ mutex_unlock(&queue->send_mutex);



clear_bit(NVME_TCP_Q_POLLING, &queue->flags);



return queue->nr_cqe;



}



 



Maurizio