* [PATCH 0/5] smb:client: fix possible use after free problems
@ 2025-08-04 12:10 Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection() Stefan Metzmacher
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-04 12:10 UTC (permalink / raw)
To: linux-cifs, samba-technical; +Cc: metze, Steve French, Tom Talpey, Long Li
While refactoring the client and server smbdirect code I
noticed a few problems where we might hit use after free
style problems.
In order to allow backports I decided to fix the problems
before trying to move things to common code.
The server has similar problems, I'll send a separate
patchset for the server as they are independed.
Stefan Metzmacher (5):
smb: client: let send_done() cleanup before calling
smbd_disconnect_rdma_connection()
smb: client: remove separate empty_packet_queue
smb: client: make sure we call ib_dma_unmap_single() only if we called
ib_dma_map_single already
smb: client: let recv_done() cleanup before notifying the callers.
smb: client: let recv_done() avoid touching data_transfer after
cleanup/move
fs/smb/client/cifs_debug.c | 6 +-
fs/smb/client/smbdirect.c | 124 ++++++++++++-------------------------
fs/smb/client/smbdirect.h | 4 --
3 files changed, 42 insertions(+), 92 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()
2025-08-04 12:10 [PATCH 0/5] smb:client: fix possible use after free problems Stefan Metzmacher
@ 2025-08-04 12:10 ` Stefan Metzmacher
2025-08-06 11:39 ` Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 2/5] smb: client: remove separate empty_packet_queue Stefan Metzmacher
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-04 12:10 UTC (permalink / raw)
To: linux-cifs, samba-technical; +Cc: metze, Steve French, Tom Talpey, Long Li
We should call ib_dma_unmap_single() and mempool_free() before calling
smbd_disconnect_rdma_connection().
And smbd_disconnect_rdma_connection() needs to be the last function to
call as all other state might already be gone after it returns.
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Long Li <longli@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
fs/smb/client/smbdirect.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index 754e94a0e07f..b6c369088479 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -281,18 +281,20 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
log_rdma_send(INFO, "smbd_request 0x%p completed wc->status=%d\n",
request, wc->status);
- if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
- log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
- wc->status, wc->opcode);
- smbd_disconnect_rdma_connection(request->info);
- }
-
for (i = 0; i < request->num_sge; i++)
ib_dma_unmap_single(sc->ib.dev,
request->sge[i].addr,
request->sge[i].length,
DMA_TO_DEVICE);
+ if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
+ log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
+ wc->status, wc->opcode);
+ mempool_free(request, request->info->request_mempool);
+ smbd_disconnect_rdma_connection(request->info);
+ return;
+ }
+
if (atomic_dec_and_test(&request->info->send_pending))
wake_up(&request->info->wait_send_pending);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/5] smb: client: remove separate empty_packet_queue
2025-08-04 12:10 [PATCH 0/5] smb:client: fix possible use after free problems Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection() Stefan Metzmacher
@ 2025-08-04 12:10 ` Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 3/5] smb: client: make sure we call ib_dma_unmap_single() only if we called ib_dma_map_single already Stefan Metzmacher
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-04 12:10 UTC (permalink / raw)
To: linux-cifs, samba-technical; +Cc: metze, Steve French, Tom Talpey, Long Li
There's no need to maintain two lists, we can just
have a single list of receive buffers, which are free to use.
It just added unneeded complexity and resulted in
ib_dma_unmap_single() not being called from recv_done()
for empty keepalive packets.
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Long Li <longli@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
fs/smb/client/cifs_debug.c | 6 ++--
fs/smb/client/smbdirect.c | 62 +++-----------------------------------
fs/smb/client/smbdirect.h | 4 ---
3 files changed, 7 insertions(+), 65 deletions(-)
diff --git a/fs/smb/client/cifs_debug.c b/fs/smb/client/cifs_debug.c
index f1cea365b6f1..ca7c2265c25f 100644
--- a/fs/smb/client/cifs_debug.c
+++ b/fs/smb/client/cifs_debug.c
@@ -481,10 +481,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
server->smbd_conn->receive_credit_target);
seq_printf(m, "\nPending send_pending: %x ",
atomic_read(&server->smbd_conn->send_pending));
- seq_printf(m, "\nReceive buffers count_receive_queue: %x "
- "count_empty_packet_queue: %x",
- server->smbd_conn->count_receive_queue,
- server->smbd_conn->count_empty_packet_queue);
+ seq_printf(m, "\nReceive buffers count_receive_queue: %x ",
+ server->smbd_conn->count_receive_queue);
seq_printf(m, "\nMR responder_resources: %x "
"max_frmr_depth: %x mr_type: %x",
server->smbd_conn->responder_resources,
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index b6c369088479..b18e2bc6c8ed 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -13,8 +13,6 @@
#include "cifsproto.h"
#include "smb2proto.h"
-static struct smbd_response *get_empty_queue_buffer(
- struct smbd_connection *info);
static struct smbd_response *get_receive_buffer(
struct smbd_connection *info);
static void put_receive_buffer(
@@ -23,8 +21,6 @@ static void put_receive_buffer(
static int allocate_receive_buffers(struct smbd_connection *info, int num_buf);
static void destroy_receive_buffers(struct smbd_connection *info);
-static void put_empty_packet(
- struct smbd_connection *info, struct smbd_response *response);
static void enqueue_reassembly(
struct smbd_connection *info,
struct smbd_response *response, int data_length);
@@ -393,7 +389,6 @@ static bool process_negotiation_response(
static void smbd_post_send_credits(struct work_struct *work)
{
int ret = 0;
- int use_receive_queue = 1;
int rc;
struct smbd_response *response;
struct smbd_connection *info =
@@ -409,18 +404,9 @@ static void smbd_post_send_credits(struct work_struct *work)
if (info->receive_credit_target >
atomic_read(&info->receive_credits)) {
while (true) {
- if (use_receive_queue)
- response = get_receive_buffer(info);
- else
- response = get_empty_queue_buffer(info);
- if (!response) {
- /* now switch to empty packet queue */
- if (use_receive_queue) {
- use_receive_queue = 0;
- continue;
- } else
- break;
- }
+ response = get_receive_buffer(info);
+ if (!response)
+ break;
response->type = SMBD_TRANSFER_DATA;
response->first_segment = false;
@@ -511,7 +497,7 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
response,
data_length);
} else
- put_empty_packet(info, response);
+ put_receive_buffer(info, response);
if (data_length)
wake_up_interruptible(&info->wait_reassembly_queue);
@@ -1115,17 +1101,6 @@ static int smbd_negotiate(struct smbd_connection *info)
return rc;
}
-static void put_empty_packet(
- struct smbd_connection *info, struct smbd_response *response)
-{
- spin_lock(&info->empty_packet_queue_lock);
- list_add_tail(&response->list, &info->empty_packet_queue);
- info->count_empty_packet_queue++;
- spin_unlock(&info->empty_packet_queue_lock);
-
- queue_work(info->workqueue, &info->post_send_credits_work);
-}
-
/*
* Implement Connection.FragmentReassemblyBuffer defined in [MS-SMBD] 3.1.1.1
* This is a queue for reassembling upper layer payload and present to upper
@@ -1174,25 +1149,6 @@ static struct smbd_response *_get_first_reassembly(struct smbd_connection *info)
return ret;
}
-static struct smbd_response *get_empty_queue_buffer(
- struct smbd_connection *info)
-{
- struct smbd_response *ret = NULL;
- unsigned long flags;
-
- spin_lock_irqsave(&info->empty_packet_queue_lock, flags);
- if (!list_empty(&info->empty_packet_queue)) {
- ret = list_first_entry(
- &info->empty_packet_queue,
- struct smbd_response, list);
- list_del(&ret->list);
- info->count_empty_packet_queue--;
- }
- spin_unlock_irqrestore(&info->empty_packet_queue_lock, flags);
-
- return ret;
-}
-
/*
* Get a receive buffer
* For each remote send, we need to post a receive. The receive buffers are
@@ -1257,10 +1213,6 @@ static int allocate_receive_buffers(struct smbd_connection *info, int num_buf)
spin_lock_init(&info->receive_queue_lock);
info->count_receive_queue = 0;
- INIT_LIST_HEAD(&info->empty_packet_queue);
- spin_lock_init(&info->empty_packet_queue_lock);
- info->count_empty_packet_queue = 0;
-
init_waitqueue_head(&info->wait_receive_queues);
for (i = 0; i < num_buf; i++) {
@@ -1294,9 +1246,6 @@ static void destroy_receive_buffers(struct smbd_connection *info)
while ((response = get_receive_buffer(info)))
mempool_free(response, info->response_mempool);
-
- while ((response = get_empty_queue_buffer(info)))
- mempool_free(response, info->response_mempool);
}
/* Implement idle connection timer [MS-SMBD] 3.1.6.2 */
@@ -1383,8 +1332,7 @@ void smbd_destroy(struct TCP_Server_Info *server)
log_rdma_event(INFO, "free receive buffers\n");
wait_event(info->wait_receive_queues,
- info->count_receive_queue + info->count_empty_packet_queue
- == sp->recv_credit_max);
+ info->count_receive_queue == sp->recv_credit_max);
destroy_receive_buffers(info);
/*
diff --git a/fs/smb/client/smbdirect.h b/fs/smb/client/smbdirect.h
index 75b3f491c3ad..ea04ce8a9763 100644
--- a/fs/smb/client/smbdirect.h
+++ b/fs/smb/client/smbdirect.h
@@ -110,10 +110,6 @@ struct smbd_connection {
int count_receive_queue;
spinlock_t receive_queue_lock;
- struct list_head empty_packet_queue;
- int count_empty_packet_queue;
- spinlock_t empty_packet_queue_lock;
-
wait_queue_head_t wait_receive_queues;
/* Reassembly queue */
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/5] smb: client: make sure we call ib_dma_unmap_single() only if we called ib_dma_map_single already
2025-08-04 12:10 [PATCH 0/5] smb:client: fix possible use after free problems Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection() Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 2/5] smb: client: remove separate empty_packet_queue Stefan Metzmacher
@ 2025-08-04 12:10 ` Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 4/5] smb: client: let recv_done() cleanup before notifying the callers Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 5/5] smb: client: let recv_done() avoid touching data_transfer after cleanup/move Stefan Metzmacher
4 siblings, 0 replies; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-04 12:10 UTC (permalink / raw)
To: linux-cifs, samba-technical; +Cc: metze, Steve French, Tom Talpey, Long Li
In case of failures either ib_dma_map_single() might not be called yet
or ib_dma_unmap_single() was already called.
We should make sure put_receive_buffer() only calls
ib_dma_unmap_single() if needed.
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Long Li <longli@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
fs/smb/client/smbdirect.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index b18e2bc6c8ed..a32ebb4d48a2 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -1057,6 +1057,7 @@ static int smbd_post_recv(
if (rc) {
ib_dma_unmap_single(sc->ib.dev, response->sge.addr,
response->sge.length, DMA_FROM_DEVICE);
+ response->sge.length = 0;
smbd_disconnect_rdma_connection(info);
log_rdma_recv(ERR, "ib_post_recv failed rc=%d\n", rc);
}
@@ -1186,8 +1187,13 @@ static void put_receive_buffer(
struct smbdirect_socket *sc = &info->socket;
unsigned long flags;
- ib_dma_unmap_single(sc->ib.dev, response->sge.addr,
- response->sge.length, DMA_FROM_DEVICE);
+ if (likely(response->sge.length != 0)) {
+ ib_dma_unmap_single(sc->ib.dev,
+ response->sge.addr,
+ response->sge.length,
+ DMA_FROM_DEVICE);
+ response->sge.length = 0;
+ }
spin_lock_irqsave(&info->receive_queue_lock, flags);
list_add_tail(&response->list, &info->receive_queue);
@@ -1221,6 +1227,7 @@ static int allocate_receive_buffers(struct smbd_connection *info, int num_buf)
goto allocate_failed;
response->info = info;
+ response->sge.length = 0;
list_add_tail(&response->list, &info->receive_queue);
info->count_receive_queue++;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/5] smb: client: let recv_done() cleanup before notifying the callers.
2025-08-04 12:10 [PATCH 0/5] smb:client: fix possible use after free problems Stefan Metzmacher
` (2 preceding siblings ...)
2025-08-04 12:10 ` [PATCH 3/5] smb: client: make sure we call ib_dma_unmap_single() only if we called ib_dma_map_single already Stefan Metzmacher
@ 2025-08-04 12:10 ` Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 5/5] smb: client: let recv_done() avoid touching data_transfer after cleanup/move Stefan Metzmacher
4 siblings, 0 replies; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-04 12:10 UTC (permalink / raw)
To: linux-cifs, samba-technical; +Cc: metze, Steve French, Tom Talpey, Long Li
We should call put_receive_buffer() before waking up the callers.
For the internal error case of response->type being unexpected,
we now also call smbd_disconnect_rdma_connection() instead
of not waking up the callers at all.
Note that the SMBD_TRANSFER_DATA case still has problems,
which will be addressed in the next commit in order to make
it easier to review this one.
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Long Li <longli@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
fs/smb/client/smbdirect.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index a32ebb4d48a2..21a12e08082f 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -454,7 +454,6 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_RECV) {
log_rdma_recv(INFO, "wc->status=%d opcode=%d\n",
wc->status, wc->opcode);
- smbd_disconnect_rdma_connection(info);
goto error;
}
@@ -471,8 +470,9 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
info->full_packet_received = true;
info->negotiate_done =
process_negotiation_response(response, wc->byte_len);
+ put_receive_buffer(info, response);
complete(&info->negotiate_completion);
- break;
+ return;
/* SMBD data transfer packet */
case SMBD_TRANSFER_DATA:
@@ -529,14 +529,16 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
}
return;
-
- default:
- log_rdma_recv(ERR,
- "unexpected response type=%d\n", response->type);
}
+ /*
+ * This is an internal error!
+ */
+ log_rdma_recv(ERR, "unexpected response type=%d\n", response->type);
+ WARN_ON_ONCE(response->type != SMBD_TRANSFER_DATA);
error:
put_receive_buffer(info, response);
+ smbd_disconnect_rdma_connection(info);
}
static struct rdma_cm_id *smbd_create_id(
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 5/5] smb: client: let recv_done() avoid touching data_transfer after cleanup/move
2025-08-04 12:10 [PATCH 0/5] smb:client: fix possible use after free problems Stefan Metzmacher
` (3 preceding siblings ...)
2025-08-04 12:10 ` [PATCH 4/5] smb: client: let recv_done() cleanup before notifying the callers Stefan Metzmacher
@ 2025-08-04 12:10 ` Stefan Metzmacher
4 siblings, 0 replies; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-04 12:10 UTC (permalink / raw)
To: linux-cifs, samba-technical; +Cc: metze, Steve French, Tom Talpey, Long Li
Calling enqueue_reassembly() and wake_up_interruptible(&info->wait_reassembly_queue)
or put_receive_buffer() means the reponse/data_transfer pointer might
get re-used by another thread, which means these should be
the last operations before calling return.
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Long Li <longli@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
fs/smb/client/smbdirect.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index 21a12e08082f..58321e483a1a 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -479,10 +479,6 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
data_transfer = smbd_response_payload(response);
data_length = le32_to_cpu(data_transfer->data_length);
- /*
- * If this is a packet with data playload place the data in
- * reassembly queue and wake up the reading thread
- */
if (data_length) {
if (info->full_packet_received)
response->first_segment = true;
@@ -491,16 +487,7 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
info->full_packet_received = false;
else
info->full_packet_received = true;
-
- enqueue_reassembly(
- info,
- response,
- data_length);
- } else
- put_receive_buffer(info, response);
-
- if (data_length)
- wake_up_interruptible(&info->wait_reassembly_queue);
+ }
atomic_dec(&info->receive_credits);
info->receive_credit_target =
@@ -528,6 +515,16 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc)
info->keep_alive_requested = KEEP_ALIVE_PENDING;
}
+ /*
+ * If this is a packet with data playload place the data in
+ * reassembly queue and wake up the reading thread
+ */
+ if (data_length) {
+ enqueue_reassembly(info, response, data_length);
+ wake_up_interruptible(&info->wait_reassembly_queue);
+ } else
+ put_receive_buffer(info, response);
+
return;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()
2025-08-04 12:10 ` [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection() Stefan Metzmacher
@ 2025-08-06 11:39 ` Stefan Metzmacher
2025-08-06 17:36 ` Steve French
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Metzmacher @ 2025-08-06 11:39 UTC (permalink / raw)
To: Steve French
Cc: Steve French, Tom Talpey, Long Li, linux-cifs@vger.kernel.org,
Samba Technical
Hi Steve,
can you please squash this into the commit? Otherwise it introduces
as new use-after-free problem with request->info.
diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
index 7377216e033d..5d1fa83583f6 100644
--- a/fs/smb/client/smbdirect.c
+++ b/fs/smb/client/smbdirect.c
@@ -286,8 +286,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
wc->status, wc->opcode);
- mempool_free(request, request->info->request_mempool);
- smbd_disconnect_rdma_connection(request->info);
+ mempool_free(request, info->request_mempool);
+ smbd_disconnect_rdma_connection(info);
return;
}
Thanks!
metze
Am 04.08.25 um 14:10 schrieb Stefan Metzmacher:
> We should call ib_dma_unmap_single() and mempool_free() before calling
> smbd_disconnect_rdma_connection().
>
> And smbd_disconnect_rdma_connection() needs to be the last function to
> call as all other state might already be gone after it returns.
>
> Cc: Steve French <smfrench@gmail.com>
> Cc: Tom Talpey <tom@talpey.com>
> Cc: Long Li <longli@microsoft.com>
> Cc: linux-cifs@vger.kernel.org
> Cc: samba-technical@lists.samba.org
> Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
> Signed-off-by: Stefan Metzmacher <metze@samba.org>
> ---
> fs/smb/client/smbdirect.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
> index 754e94a0e07f..b6c369088479 100644
> --- a/fs/smb/client/smbdirect.c
> +++ b/fs/smb/client/smbdirect.c
> @@ -281,18 +281,20 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
> log_rdma_send(INFO, "smbd_request 0x%p completed wc->status=%d\n",
> request, wc->status);
>
> - if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> - log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> - wc->status, wc->opcode);
> - smbd_disconnect_rdma_connection(request->info);
> - }
> -
> for (i = 0; i < request->num_sge; i++)
> ib_dma_unmap_single(sc->ib.dev,
> request->sge[i].addr,
> request->sge[i].length,
> DMA_TO_DEVICE);
>
> + if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> + log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> + wc->status, wc->opcode);
> + mempool_free(request, request->info->request_mempool);
> + smbd_disconnect_rdma_connection(request->info);
> + return;
> + }
> +
> if (atomic_dec_and_test(&request->info->send_pending))
> wake_up(&request->info->wait_send_pending);
>
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection()
2025-08-06 11:39 ` Stefan Metzmacher
@ 2025-08-06 17:36 ` Steve French
0 siblings, 0 replies; 8+ messages in thread
From: Steve French @ 2025-08-06 17:36 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: Steve French, Tom Talpey, Long Li, linux-cifs@vger.kernel.org,
Samba Technical
Done
On Wed, Aug 6, 2025 at 6:39 AM Stefan Metzmacher <metze@samba.org> wrote:
>
> Hi Steve,
>
> can you please squash this into the commit? Otherwise it introduces
> as new use-after-free problem with request->info.
>
> diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
> index 7377216e033d..5d1fa83583f6 100644
> --- a/fs/smb/client/smbdirect.c
> +++ b/fs/smb/client/smbdirect.c
> @@ -286,8 +286,8 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
> if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> wc->status, wc->opcode);
> - mempool_free(request, request->info->request_mempool);
> - smbd_disconnect_rdma_connection(request->info);
> + mempool_free(request, info->request_mempool);
> + smbd_disconnect_rdma_connection(info);
> return;
> }
>
> Thanks!
> metze
>
> Am 04.08.25 um 14:10 schrieb Stefan Metzmacher:
> > We should call ib_dma_unmap_single() and mempool_free() before calling
> > smbd_disconnect_rdma_connection().
> >
> > And smbd_disconnect_rdma_connection() needs to be the last function to
> > call as all other state might already be gone after it returns.
> >
> > Cc: Steve French <smfrench@gmail.com>
> > Cc: Tom Talpey <tom@talpey.com>
> > Cc: Long Li <longli@microsoft.com>
> > Cc: linux-cifs@vger.kernel.org
> > Cc: samba-technical@lists.samba.org
> > Fixes: f198186aa9bb ("CIFS: SMBD: Establish SMB Direct connection")
> > Signed-off-by: Stefan Metzmacher <metze@samba.org>
> > ---
> > fs/smb/client/smbdirect.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/smb/client/smbdirect.c b/fs/smb/client/smbdirect.c
> > index 754e94a0e07f..b6c369088479 100644
> > --- a/fs/smb/client/smbdirect.c
> > +++ b/fs/smb/client/smbdirect.c
> > @@ -281,18 +281,20 @@ static void send_done(struct ib_cq *cq, struct ib_wc *wc)
> > log_rdma_send(INFO, "smbd_request 0x%p completed wc->status=%d\n",
> > request, wc->status);
> >
> > - if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> > - log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> > - wc->status, wc->opcode);
> > - smbd_disconnect_rdma_connection(request->info);
> > - }
> > -
> > for (i = 0; i < request->num_sge; i++)
> > ib_dma_unmap_single(sc->ib.dev,
> > request->sge[i].addr,
> > request->sge[i].length,
> > DMA_TO_DEVICE);
> >
> > + if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_SEND) {
> > + log_rdma_send(ERR, "wc->status=%d wc->opcode=%d\n",
> > + wc->status, wc->opcode);
> > + mempool_free(request, request->info->request_mempool);
> > + smbd_disconnect_rdma_connection(request->info);
> > + return;
> > + }
> > +
> > if (atomic_dec_and_test(&request->info->send_pending))
> > wake_up(&request->info->wait_send_pending);
> >
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-08-06 17:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-04 12:10 [PATCH 0/5] smb:client: fix possible use after free problems Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 1/5] smb: client: let send_done() cleanup before calling smbd_disconnect_rdma_connection() Stefan Metzmacher
2025-08-06 11:39 ` Stefan Metzmacher
2025-08-06 17:36 ` Steve French
2025-08-04 12:10 ` [PATCH 2/5] smb: client: remove separate empty_packet_queue Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 3/5] smb: client: make sure we call ib_dma_unmap_single() only if we called ib_dma_map_single already Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 4/5] smb: client: let recv_done() cleanup before notifying the callers Stefan Metzmacher
2025-08-04 12:10 ` [PATCH 5/5] smb: client: let recv_done() avoid touching data_transfer after cleanup/move Stefan Metzmacher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).