--
Best regards,
Pavel Shilovsky
ср, 23 янв. 2019 г. в 13:10, Pavel Shilovsky <piastryyy@xxxxxxxxx>:
>
> When executing add_credits() we currently call cifs_reconnect()
> if the number of credits is zero and there are no requests in
> flight. In this case we may call cifs_reconnect() recursively
> twice and cause memory corruption given the following sequence
> of functions:
>
> mid1.callback() -> add_credits() -> cifs_reconnect() ->
> -> mid2.callback() -> add_credits() -> cifs_reconnect().
>
> Fix this by avoiding to call cifs_reconnect() in add_credits()
> and checking for zero credits in the demultiplex thread.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> ---
> fs/cifs/connect.c | 21 +++++++++++++++++++++
> fs/cifs/smb2ops.c | 29 ++++++++++++++++++++++-------
> 2 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 3d5e308..dc02b6d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -699,6 +699,21 @@ server_unresponsive(struct TCP_Server_Info *server)
> return false;
> }
>
> +static inline bool
> +zero_credits(struct TCP_Server_Info *server)
> +{
> + int val;
> +
> + spin_lock(&server->req_lock);
> + val = server->credits + server->echo_credits + server->oplock_credits;
> + if (server->in_flight == 0 && val == 0) {
> + spin_unlock(&server->req_lock);
> + return true;
> + }
> + spin_unlock(&server->req_lock);
> + return false;
> +}
> +
> static int
> cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
> {
> @@ -711,6 +726,12 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
> for (total_read = 0; msg_data_left(smb_msg); total_read += length) {
> try_to_freeze();
>
> + /* reconnect if no credits and no requests in flight */
> + if (zero_credits(server)) {
> + cifs_reconnect(server);
> + return -ECONNABORTED;
> + }
> +
> if (server_unresponsive(server))
> return -ECONNABORTED;
> if (cifs_rdma_enabled(server) && server->smbd_conn)
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 82967f6..3b6c95d 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -34,6 +34,7 @@
> #include "cifs_ioctl.h"
> #include "smbdirect.h"
>
> +/* Change credits for different ops and return the total number of credits */
> static int
> change_conf(struct TCP_Server_Info *server)
> {
> @@ -41,17 +42,15 @@ change_conf(struct TCP_Server_Info *server)
> server->oplock_credits = server->echo_credits = 0;
> switch (server->credits) {
> case 0:
> - return -1;
> + return 0;
> case 1:
> server->echoes = false;
> server->oplocks = false;
> - cifs_dbg(VFS, "disabling echoes and oplocks\n");
> break;
> case 2:
> server->echoes = true;
> server->oplocks = false;
> server->echo_credits = 1;
> - cifs_dbg(FYI, "disabling oplocks\n");
> break;
> default:
> server->echoes = true;
> @@ -64,14 +63,15 @@ change_conf(struct TCP_Server_Info *server)
> server->echo_credits = 1;
> }
> server->credits -= server->echo_credits + server->oplock_credits;
> - return 0;
> + return server->credits + server->echo_credits + server->oplock_credits;
> }
>
> static void
> smb2_add_credits(struct TCP_Server_Info *server, const unsigned int add,
> const int optype)
> {
> - int *val, rc = 0;
> + int *val, rc = -1;
> +
> spin_lock(&server->req_lock);
> val = server->ops->get_credits_field(server, optype);
>
> @@ -101,8 +101,23 @@ smb2_add_credits(struct TCP_Server_Info *server, const unsigned int add,
> }
> spin_unlock(&server->req_lock);
> wake_up(&server->request_q);
> - if (rc)
> - cifs_reconnect(server);
We should prevent logging if we are reconnecting to not confuse users:
+
+ if (server->tcpStatus == CifsNeedReconnect)
+ return;
> +
> + switch (rc) {
> + case -1:
> + /* change_conf hasn't been executed */
> + break;
> + case 0:
> + cifs_dbg(VFS, "Possible client or server bug - zero credits\n");
> + break;
> + case 1:
> + cifs_dbg(VFS, "disabling echoes and oplocks\n");
> + break;
> + case 2:
> + cifs_dbg(FYI, "disabling oplocks\n");
> + break;
> + default:
> + cifs_dbg(FYI, "add %u credits total=%d\n", add, rc);
> + }
> }
>
> static void
> --
> 2.7.4
>
