From 49ca2147749fb69e1caa0f56a98bec065d903bd0 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 2 Dec 2025 22:15:24 +0100 Subject: [PATCH 1/7] smb: smbdirect: introduce smbdirect_socket.connect.{lock,work} This will first be used by the server in order to defer the processing of the initial recv of the negotiation request. But in future it will also be used by the client in order to implement an async connect. Cc: Tom Talpey Cc: Long Li Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Signed-off-by: Stefan Metzmacher Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/common/smbdirect/smbdirect_socket.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/smb/common/smbdirect/smbdirect_socket.h b/fs/smb/common/smbdirect/smbdirect_socket.h index 384b19177e1c..ee4c2726771a 100644 --- a/fs/smb/common/smbdirect/smbdirect_socket.h +++ b/fs/smb/common/smbdirect/smbdirect_socket.h @@ -132,6 +132,14 @@ struct smbdirect_socket { struct smbdirect_socket_parameters parameters; + /* + * The state for connect/negotiation + */ + struct { + spinlock_t lock; + struct work_struct work; + } connect; + /* * The state for keepalive and timeout handling */ @@ -353,6 +361,10 @@ static __always_inline void smbdirect_socket_init(struct smbdirect_socket *sc) INIT_WORK(&sc->disconnect_work, __smbdirect_socket_disabled_work); disable_work_sync(&sc->disconnect_work); + spin_lock_init(&sc->connect.lock); + INIT_WORK(&sc->connect.work, __smbdirect_socket_disabled_work); + disable_work_sync(&sc->connect.work); + INIT_WORK(&sc->idle.immediate_work, __smbdirect_socket_disabled_work); disable_work_sync(&sc->idle.immediate_work); INIT_DELAYED_WORK(&sc->idle.timer_work, __smbdirect_socket_disabled_work); From c1fb124f2a7416905047cf36fa6a110f9c48cd02 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 2 Dec 2025 22:15:25 +0100 Subject: [PATCH 2/7] smb: server: initialize recv_io->cqe.done = recv_done just once smbdirect_recv_io structures are pre-allocated so we can set the callback function just once. This will make it easy to move smb_direct_post_recv to common code soon. Cc: Tom Talpey Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Signed-off-by: Stefan Metzmacher Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/transport_rdma.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/smb/server/transport_rdma.c b/fs/smb/server/transport_rdma.c index 4e7ab8d9314f..222d1b5365e8 100644 --- a/fs/smb/server/transport_rdma.c +++ b/fs/smb/server/transport_rdma.c @@ -758,7 +758,6 @@ static int smb_direct_post_recv(struct smbdirect_socket *sc, return ret; recvmsg->sge.length = sp->max_recv_size; recvmsg->sge.lkey = sc->ib.pd->local_dma_lkey; - recvmsg->cqe.done = recv_done; wr.wr_cqe = &recvmsg->cqe; wr.next = NULL; @@ -2339,6 +2338,7 @@ respond: static int smb_direct_connect(struct smbdirect_socket *sc) { + struct smbdirect_recv_io *recv_io; int ret; ret = smb_direct_init_params(sc); @@ -2353,6 +2353,9 @@ static int smb_direct_connect(struct smbdirect_socket *sc) return ret; } + list_for_each_entry(recv_io, &sc->recv_io.free.list, list) + recv_io->cqe.done = recv_done; + ret = smb_direct_create_qpair(sc); if (ret) { pr_err("Can't accept RDMA client: %d\n", ret); From d180b1d9c7a401656332b27e3428a949c00748d3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 2 Dec 2025 22:15:26 +0100 Subject: [PATCH 3/7] smb: server: defer the initial recv completion logic to smb_direct_negotiate_recv_work() The previous change to relax WARN_ON_ONCE(SMBDIRECT_SOCKET_*) checks in recv_done() and smb_direct_cm_handler() seems to work around the problem that the order of initial recv completion and RDMA_CM_EVENT_ESTABLISHED is random, but it's still a bit ugly. This implements a better solution deferring the recv completion processing to smb_direct_negotiate_recv_work(), which is queued only if both events arrived. In order to avoid more basic changes to the main recv_done callback, I introduced a smb_direct_negotiate_recv_done, which is only used for the first pdu, this will allow further cleanup and simplifications in recv_done as a future patch. smb_direct_negotiate_recv_work() is also very basic with only basic error checking and the transition from SMBDIRECT_SOCKET_NEGOTIATE_NEEDED to SMBDIRECT_SOCKET_NEGOTIATE_RUNNING, which allows smb_direct_prepare() to continue as before. Cc: Tom Talpey Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Signed-off-by: Stefan Metzmacher Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/transport_rdma.c | 170 +++++++++++++++++++++++++++------ 1 file changed, 142 insertions(+), 28 deletions(-) diff --git a/fs/smb/server/transport_rdma.c b/fs/smb/server/transport_rdma.c index 222d1b5365e8..f585359684d4 100644 --- a/fs/smb/server/transport_rdma.c +++ b/fs/smb/server/transport_rdma.c @@ -242,6 +242,7 @@ static void smb_direct_disconnect_rdma_work(struct work_struct *work) * disable[_delayed]_work_sync() */ disable_work(&sc->disconnect_work); + disable_work(&sc->connect.work); disable_work(&sc->recv_io.posted.refill_work); disable_delayed_work(&sc->idle.timer_work); disable_work(&sc->idle.immediate_work); @@ -297,6 +298,7 @@ smb_direct_disconnect_rdma_connection(struct smbdirect_socket *sc) * not queued again but here we don't block and avoid * disable[_delayed]_work_sync() */ + disable_work(&sc->connect.work); disable_work(&sc->recv_io.posted.refill_work); disable_work(&sc->idle.immediate_work); disable_delayed_work(&sc->idle.timer_work); @@ -467,6 +469,7 @@ static void free_transport(struct smb_direct_transport *t) */ smb_direct_disconnect_wake_up_all(sc); + disable_work_sync(&sc->connect.work); disable_work_sync(&sc->recv_io.posted.refill_work); disable_delayed_work_sync(&sc->idle.timer_work); disable_work_sync(&sc->idle.immediate_work); @@ -635,28 +638,8 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) switch (sc->recv_io.expected) { case SMBDIRECT_EXPECT_NEGOTIATE_REQ: - if (wc->byte_len < sizeof(struct smbdirect_negotiate_req)) { - put_recvmsg(sc, recvmsg); - smb_direct_disconnect_rdma_connection(sc); - return; - } - sc->recv_io.reassembly.full_packet_received = true; - /* - * Some drivers (at least mlx5_ib) might post a - * recv completion before RDMA_CM_EVENT_ESTABLISHED, - * we need to adjust our expectation in that case. - */ - if (!sc->first_error && sc->status == SMBDIRECT_SOCKET_RDMA_CONNECT_RUNNING) - sc->status = SMBDIRECT_SOCKET_NEGOTIATE_NEEDED; - if (SMBDIRECT_CHECK_STATUS_WARN(sc, SMBDIRECT_SOCKET_NEGOTIATE_NEEDED)) { - put_recvmsg(sc, recvmsg); - smb_direct_disconnect_rdma_connection(sc); - return; - } - sc->status = SMBDIRECT_SOCKET_NEGOTIATE_RUNNING; - enqueue_reassembly(sc, recvmsg, 0); - wake_up(&sc->status_wait); - return; + /* see smb_direct_negotiate_recv_done */ + break; case SMBDIRECT_EXPECT_DATA_TRANSFER: { struct smbdirect_data_transfer *data_transfer = (struct smbdirect_data_transfer *)recvmsg->packet; @@ -742,6 +725,126 @@ static void recv_done(struct ib_cq *cq, struct ib_wc *wc) smb_direct_disconnect_rdma_connection(sc); } +static void smb_direct_negotiate_recv_work(struct work_struct *work); + +static void smb_direct_negotiate_recv_done(struct ib_cq *cq, struct ib_wc *wc) +{ + struct smbdirect_recv_io *recv_io = + container_of(wc->wr_cqe, struct smbdirect_recv_io, cqe); + struct smbdirect_socket *sc = recv_io->socket; + unsigned long flags; + + /* + * reset the common recv_done for later reuse. + */ + recv_io->cqe.done = recv_done; + + if (wc->status != IB_WC_SUCCESS || wc->opcode != IB_WC_RECV) { + put_recvmsg(sc, recv_io); + if (wc->status != IB_WC_WR_FLUSH_ERR) { + pr_err("Negotiate Recv error. status='%s (%d)' opcode=%d\n", + ib_wc_status_msg(wc->status), wc->status, + wc->opcode); + smb_direct_disconnect_rdma_connection(sc); + } + return; + } + + ksmbd_debug(RDMA, "Negotiate Recv completed. status='%s (%d)', opcode=%d\n", + ib_wc_status_msg(wc->status), wc->status, + wc->opcode); + + ib_dma_sync_single_for_cpu(sc->ib.dev, + recv_io->sge.addr, + recv_io->sge.length, + DMA_FROM_DEVICE); + + /* + * This is an internal error! + */ + if (WARN_ON_ONCE(sc->recv_io.expected != SMBDIRECT_EXPECT_NEGOTIATE_REQ)) { + put_recvmsg(sc, recv_io); + smb_direct_disconnect_rdma_connection(sc); + return; + } + + /* + * Don't reset timer to the keepalive interval in + * this will be done in smb_direct_negotiate_recv_work. + */ + + /* + * Only remember the recv_io if it has enough bytes, + * this gives smb_direct_negotiate_recv_work enough + * information in order to disconnect if it was not + * valid. + */ + sc->recv_io.reassembly.full_packet_received = true; + if (wc->byte_len >= sizeof(struct smbdirect_negotiate_req)) + enqueue_reassembly(sc, recv_io, 0); + else + put_recvmsg(sc, recv_io); + + /* + * Some drivers (at least mlx5_ib and irdma in roce mode) + * might post a recv completion before RDMA_CM_EVENT_ESTABLISHED, + * we need to adjust our expectation in that case. + * + * So we defer further processing of the negotiation + * to smb_direct_negotiate_recv_work(). + * + * If we are already in SMBDIRECT_SOCKET_NEGOTIATE_NEEDED + * we queue the work directly otherwise + * smb_direct_cm_handler() will do it, when + * RDMA_CM_EVENT_ESTABLISHED arrived. + */ + spin_lock_irqsave(&sc->connect.lock, flags); + if (!sc->first_error) { + INIT_WORK(&sc->connect.work, smb_direct_negotiate_recv_work); + if (sc->status == SMBDIRECT_SOCKET_NEGOTIATE_NEEDED) + queue_work(sc->workqueue, &sc->connect.work); + } + spin_unlock_irqrestore(&sc->connect.lock, flags); +} + +static void smb_direct_negotiate_recv_work(struct work_struct *work) +{ + struct smbdirect_socket *sc = + container_of(work, struct smbdirect_socket, connect.work); + const struct smbdirect_socket_parameters *sp = &sc->parameters; + struct smbdirect_recv_io *recv_io; + + if (sc->first_error) + return; + + ksmbd_debug(RDMA, "Negotiate Recv Work running\n"); + + /* + * Reset timer to the keepalive interval in + * order to trigger our next keepalive message. + */ + sc->idle.keepalive = SMBDIRECT_KEEPALIVE_NONE; + mod_delayed_work(sc->workqueue, &sc->idle.timer_work, + msecs_to_jiffies(sp->keepalive_interval_msec)); + + /* + * If smb_direct_negotiate_recv_done() detected an + * invalid request we want to disconnect. + */ + recv_io = get_first_reassembly(sc); + if (!recv_io) { + smb_direct_disconnect_rdma_connection(sc); + return; + } + + if (SMBDIRECT_CHECK_STATUS_WARN(sc, SMBDIRECT_SOCKET_NEGOTIATE_NEEDED)) { + smb_direct_disconnect_rdma_connection(sc); + return; + } + sc->status = SMBDIRECT_SOCKET_NEGOTIATE_RUNNING; + wake_up(&sc->status_wait); +} + static int smb_direct_post_recv(struct smbdirect_socket *sc, struct smbdirect_recv_io *recvmsg) { @@ -1731,6 +1834,7 @@ static int smb_direct_cm_handler(struct rdma_cm_id *cm_id, struct rdma_cm_event *event) { struct smbdirect_socket *sc = cm_id->context; + unsigned long flags; ksmbd_debug(RDMA, "RDMA CM event. cm_id=%p event=%s (%d)\n", cm_id, rdma_event_msg(event->event), event->event); @@ -1738,18 +1842,27 @@ static int smb_direct_cm_handler(struct rdma_cm_id *cm_id, switch (event->event) { case RDMA_CM_EVENT_ESTABLISHED: { /* - * Some drivers (at least mlx5_ib) might post a - * recv completion before RDMA_CM_EVENT_ESTABLISHED, + * Some drivers (at least mlx5_ib and irdma in roce mode) + * might post a recv completion before RDMA_CM_EVENT_ESTABLISHED, * we need to adjust our expectation in that case. * - * As we already started the negotiation, we just - * ignore RDMA_CM_EVENT_ESTABLISHED here. + * If smb_direct_negotiate_recv_done was called first + * it initialized sc->connect.work only for us to + * start, so that we turned into + * SMBDIRECT_SOCKET_NEGOTIATE_NEEDED, before + * smb_direct_negotiate_recv_work() runs. + * + * If smb_direct_negotiate_recv_done didn't happen + * yet. sc->connect.work is still be disabled and + * queue_work() is a no-op. */ - if (!sc->first_error && sc->status > SMBDIRECT_SOCKET_RDMA_CONNECT_RUNNING) - break; if (SMBDIRECT_CHECK_STATUS_DISCONNECT(sc, SMBDIRECT_SOCKET_RDMA_CONNECT_RUNNING)) break; sc->status = SMBDIRECT_SOCKET_NEGOTIATE_NEEDED; + spin_lock_irqsave(&sc->connect.lock, flags); + if (!sc->first_error) + queue_work(sc->workqueue, &sc->connect.work); + spin_unlock_irqrestore(&sc->connect.lock, flags); wake_up(&sc->status_wait); break; } @@ -1920,6 +2033,7 @@ static int smb_direct_prepare_negotiation(struct smbdirect_socket *sc) recvmsg = get_free_recvmsg(sc); if (!recvmsg) return -ENOMEM; + recvmsg->cqe.done = smb_direct_negotiate_recv_done; ret = smb_direct_post_recv(sc, recvmsg); if (ret) { From 0446356e9f29d81757dc64ae7c61743e28d91ac0 Mon Sep 17 00:00:00 2001 From: Chen Ni Date: Tue, 18 Nov 2025 09:32:29 +0800 Subject: [PATCH 4/7] ksmbd: convert comma to semicolon Replace comma between expressions with semicolons. Using a ',' in place of a ';' can have unintended side effects. Although that is not the case here, it is seems best to use ';' unless ',' is intended. Found by inspection. No functional change intended. Compile tested only. Signed-off-by: Chen Ni Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/vfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 98b0eb966d91..f891344bd76b 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -702,7 +702,7 @@ retry: rd.old_parent = NULL; rd.new_parent = new_path.dentry; rd.flags = flags; - rd.delegated_inode = NULL, + rd.delegated_inode = NULL; err = start_renaming_dentry(&rd, lookup_flags, old_child, &new_last); if (err) goto out_drop_write; From 8dd2e58b62731a96e276ee0545fb910ffb2057d9 Mon Sep 17 00:00:00 2001 From: Alexey Velichayshiy Date: Wed, 10 Dec 2025 16:51:33 +0300 Subject: [PATCH 5/7] ksmbd: remove redundant DACL check in smb_check_perm_dacl A zero value of pdacl->num_aces is already handled at the start of smb_check_perm_dacl() so the second check is useless. Drop the unreachable code block, no functional impact intended. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Alexey Velichayshiy Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smbacl.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c index 5aa7a66334d9..05598d994a68 100644 --- a/fs/smb/server/smbacl.c +++ b/fs/smb/server/smbacl.c @@ -1307,9 +1307,6 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, granted |= le32_to_cpu(ace->access_req); ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size)); } - - if (!pdacl->num_aces) - granted = GENERIC_ALL_FLAGS; } if (!uid) From cafb57f7bdd57abba87725eb4e82bbdca4959644 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Sun, 14 Dec 2025 15:05:56 +0900 Subject: [PATCH 6/7] ksmbd: Fix refcount leak when invalid session is found on session lookup When a session is found but its state is not SMB2_SESSION_VALID, It indicates that no valid session was found, but it is missing to decrement the reference count acquired by the session lookup, which results in a reference count leak. This patch fixes the issue by explicitly calling ksmbd_user_session_put to release the reference to the session. Cc: stable@vger.kernel.org Reported-by: Alexandre Reported-by: Stanislas Polu Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/mgmt/user_session.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/smb/server/mgmt/user_session.c b/fs/smb/server/mgmt/user_session.c index 1c181ef99929..7d880ff34402 100644 --- a/fs/smb/server/mgmt/user_session.c +++ b/fs/smb/server/mgmt/user_session.c @@ -325,8 +325,10 @@ struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn, sess = ksmbd_session_lookup(conn, id); if (!sess && conn->binding) sess = ksmbd_session_lookup_slowpath(id); - if (sess && sess->state != SMB2_SESSION_VALID) + if (sess && sess->state != SMB2_SESSION_VALID) { + ksmbd_user_session_put(sess); sess = NULL; + } return sess; } From 95d7a890e4b03e198836d49d699408fd1867cb55 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Sun, 14 Dec 2025 15:06:34 +0900 Subject: [PATCH 7/7] ksmbd: fix buffer validation by including null terminator size in EA length The smb2_set_ea function, which handles Extended Attributes (EA), was performing buffer validation checks that incorrectly omitted the size of the null terminating character (+1 byte) for EA Name. This patch fixes the issue by explicitly adding '+ 1' to EaNameLength where the null terminator is expected to be present in the buffer, ensuring the validation accurately reflects the total required buffer size. Cc: stable@vger.kernel.org Reported-by: Roger Reported-by: Stanislas Polu Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 27f87a13f20a..8aa483800014 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -2363,7 +2363,7 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len, int rc = 0; unsigned int next = 0; - if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength + + if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength + 1 + le16_to_cpu(eabuf->EaValueLength)) return -EINVAL; @@ -2440,7 +2440,7 @@ next: break; } - if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength + + if (buf_len < sizeof(struct smb2_ea_info) + eabuf->EaNameLength + 1 + le16_to_cpu(eabuf->EaValueLength)) { rc = -EINVAL; break;