smb: server: call smb_direct_post_recv_credits() when the negotiation is done

We now activate sc->recv_io.posted.refill_work and sc->idle.immediate_work
only after a successful negotiation, before sending the negotiation
response.

It means the queue_work(sc->workqueue, &sc->recv_io.posted.refill_work)
in put_recvmsg() of the negotiate request, is a no-op now.

It also means our explicit smb_direct_post_recv_credits() will
have queue_work(sc->workqueue, &sc->idle.immediate_work) as no-op.

This should make sure we don't have races and post any immediate
data_transfer message that tries to grant credits to the peer,
before we send the negotiation response, as that will grant
the initial credits to the peer.

Fixes: 0626e6641f6b ("cifsd: add server handler for central processing and tranport layers")
Fixes: 1cde0a74a7a8 ("smb: server: don't use delayed_work for post_recv_credits_work")
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/transport_rdma.c b/fs/smb/server/transport_rdma.c
index 89b02efdba0c19d8003b69081a70c7da0b649720..e70fc447e815a32b0abbd43653602614ce7d13cb 100644 (file)
--- a/fs/smb/server/transport_rdma.c
+++ b/fs/smb/server/transport_rdma.c
@@ -418,9 +418,6 @@ static struct smb_direct_transport *alloc_transport(struct rdma_cm_id *cm_id)
 
        sc->ib.dev = sc->rdma.cm_id->device;
 
-       INIT_WORK(&sc->recv_io.posted.refill_work,
-                 smb_direct_post_recv_credits);
-       INIT_WORK(&sc->idle.immediate_work, smb_direct_send_immediate_work);
        INIT_DELAYED_WORK(&sc->idle.timer_work, smb_direct_idle_connection_timer);
 
        conn = ksmbd_conn_alloc();
@@ -1904,7 +1901,6 @@ static int smb_direct_prepare_negotiation(struct smbdirect_socket *sc)
                goto out_err;
        }
 
-       smb_direct_post_recv_credits(&sc->recv_io.posted.refill_work);
        return 0;
 out_err:
        put_recvmsg(sc, recvmsg);
@@ -2249,8 +2245,8 @@ static int smb_direct_prepare(struct ksmbd_transport *t)
                return -ECONNABORTED;
 
        ret = smb_direct_check_recvmsg(recvmsg);
-       if (ret == -ECONNABORTED)
-               goto out;
+       if (ret)
+               goto put;
 
        req = (struct smbdirect_negotiate_req *)recvmsg->packet;
        sp->max_recv_size = min_t(int, sp->max_recv_size,
@@ -2265,14 +2261,38 @@ static int smb_direct_prepare(struct ksmbd_transport *t)
        sc->recv_io.credits.target = min_t(u16, sc->recv_io.credits.target, sp->recv_credit_max);
        sc->recv_io.credits.target = max_t(u16, sc->recv_io.credits.target, 1);
 
-       ret = smb_direct_send_negotiate_response(sc, ret);
-out:
+put:
        spin_lock_irqsave(&sc->recv_io.reassembly.lock, flags);
        sc->recv_io.reassembly.queue_length--;
        list_del(&recvmsg->list);
        spin_unlock_irqrestore(&sc->recv_io.reassembly.lock, flags);
        put_recvmsg(sc, recvmsg);
 
+       if (ret == -ECONNABORTED)
+               return ret;
+
+       if (ret)
+               goto respond;
+
+       /*
+        * We negotiated with success, so we need to refill the recv queue.
+        * We do that with sc->idle.immediate_work still being disabled
+        * via smbdirect_socket_init(), so that queue_work(sc->workqueue,
+        * &sc->idle.immediate_work) in smb_direct_post_recv_credits()
+        * is a no-op.
+        *
+        * The message that grants the credits to the client is
+        * the negotiate response.
+        */
+       INIT_WORK(&sc->recv_io.posted.refill_work, smb_direct_post_recv_credits);
+       smb_direct_post_recv_credits(&sc->recv_io.posted.refill_work);
+       if (unlikely(sc->first_error))
+               return sc->first_error;
+       INIT_WORK(&sc->idle.immediate_work, smb_direct_send_immediate_work);
+
+respond:
+       ret = smb_direct_send_negotiate_response(sc, ret);
+
        return ret;
 }