[PATCH] cifs: add server conn_id to fscache client cookie

[PATCH] cifs: add server conn_id to fscache client cookie

[Shyam Prasad N]

The fscache client cookie uses the server address
(and port) as the cookie key. This is a problem when
nosharesock is used. Two different connections will
use duplicate cookies. Avoid this by adding
server->conn_id to the key, so that it's guaranteed
that cookie will not be duplicated.

Also, for secondary channels of a session, copy the
fscache pointer from the primary channel. The primary
channel is guaranteed not to go away as long as secondary
channels are in use.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
 fs/cifs/connect.c |  2 ++
 fs/cifs/fscache.c | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index eee994b0925f..d8822e835cc4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1562,6 +1562,8 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
  /* fscache server cookies are based on primary channel only */
  if (!CIFS_SERVER_IS_CHAN(tcp_ses))
  cifs_fscache_get_client_cookie(tcp_ses);
+ else
+ tcp_ses->fscache = tcp_ses->primary_server->fscache;

  /* queue echo request delayed work */
  queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index f4da693760c1..1db3437f3b7d 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -24,6 +24,7 @@ struct cifs_server_key {
  struct in_addr ipv4_addr;
  struct in6_addr ipv6_addr;
  };
+ __u64 conn_id;
 } __packed;

 /*
@@ -37,6 +38,14 @@ void cifs_fscache_get_client_cookie(struct
TCP_Server_Info *server)
  struct cifs_server_key key;
  uint16_t key_len = sizeof(key.hdr);

+ /*
+ * Check if cookie was already initialized so don't reinitialize it.
+ * In the future, as we integrate with newer fscache features,
+ * we may want to instead add a check if cookie has changed
+ */
+ if (server->fscache)
+ return;
+
  memset(&key, 0, sizeof(key));

  /*
@@ -62,6 +71,7 @@ void cifs_fscache_get_client_cookie(struct
TCP_Server_Info *server)
  server->fscache = NULL;
  return;
  }
+ key.conn_id = server->conn_id;

  server->fscache =
  fscache_acquire_cookie(cifs_fscache_netfs.primary_index,

Re: [PATCH] cifs: add server conn_id to fscache client cookie

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 2577 bytes --]

Added fix for problem (missing ifdef) pointed out by kernel test robot
+#ifdef CONFIG_CIFS_FSCACHE
+       else
+               tcp_ses->fscache = tcp_ses->primary_server->fscache;
+#endif /* CONFIG_CIFS_FSCACHE */



See attached

On Fri, Dec 3, 2021 at 3:28 AM Shyam Prasad N <nspmangalore@gmail.com> wrote:
>
> The fscache client cookie uses the server address
> (and port) as the cookie key. This is a problem when
> nosharesock is used. Two different connections will
> use duplicate cookies. Avoid this by adding
> server->conn_id to the key, so that it's guaranteed
> that cookie will not be duplicated.
>
> Also, for secondary channels of a session, copy the
> fscache pointer from the primary channel. The primary
> channel is guaranteed not to go away as long as secondary
> channels are in use.
>
> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> ---
>  fs/cifs/connect.c |  2 ++
>  fs/cifs/fscache.c | 10 ++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index eee994b0925f..d8822e835cc4 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1562,6 +1562,8 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
>   /* fscache server cookies are based on primary channel only */
>   if (!CIFS_SERVER_IS_CHAN(tcp_ses))
>   cifs_fscache_get_client_cookie(tcp_ses);
> + else
> + tcp_ses->fscache = tcp_ses->primary_server->fscache;
>
>   /* queue echo request delayed work */
>   queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
> diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
> index f4da693760c1..1db3437f3b7d 100644
> --- a/fs/cifs/fscache.c
> +++ b/fs/cifs/fscache.c
> @@ -24,6 +24,7 @@ struct cifs_server_key {
>   struct in_addr ipv4_addr;
>   struct in6_addr ipv6_addr;
>   };
> + __u64 conn_id;
>  } __packed;
>
>  /*
> @@ -37,6 +38,14 @@ void cifs_fscache_get_client_cookie(struct
> TCP_Server_Info *server)
>   struct cifs_server_key key;
>   uint16_t key_len = sizeof(key.hdr);
>
> + /*
> + * Check if cookie was already initialized so don't reinitialize it.
> + * In the future, as we integrate with newer fscache features,
> + * we may want to instead add a check if cookie has changed
> + */
> + if (server->fscache)
> + return;
> +
>   memset(&key, 0, sizeof(key));
>
>   /*
> @@ -62,6 +71,7 @@ void cifs_fscache_get_client_cookie(struct
> TCP_Server_Info *server)
>   server->fscache = NULL;
>   return;
>   }
> + key.conn_id = server->conn_id;
>
>   server->fscache =
>   fscache_acquire_cookie(cifs_fscache_netfs.primary_index,



-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-add-server-conn_id-to-fscache-client-cookie.patch --]
[-- Type: text/x-patch, Size: 2636 bytes --]

From 2adc82006bcb067523bedd38e93711c80fd274c1 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Thu, 2 Dec 2021 07:30:00 +0000
Subject: [PATCH] cifs: add server conn_id to fscache client cookie

The fscache client cookie uses the server address
(and port) as the cookie key. This is a problem when
nosharesock is used. Two different connections will
use duplicate cookies. Avoid this by adding
server->conn_id to the key, so that it's guaranteed
that cookie will not be duplicated.

Also, for secondary channels of a session, copy the
fscache pointer from the primary channel. The primary
channel is guaranteed not to go away as long as secondary
channels are in use.  Also addresses minor problem found
by kernel test robot.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/connect.c |  4 ++++
 fs/cifs/fscache.c | 10 ++++++++++
 2 files changed, 14 insertions(+)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7cc469e4682a..18448dbd762a 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1562,6 +1562,10 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
 	/* fscache server cookies are based on primary channel only */
 	if (!CIFS_SERVER_IS_CHAN(tcp_ses))
 		cifs_fscache_get_client_cookie(tcp_ses);
+#ifdef CONFIG_CIFS_FSCACHE
+	else
+		tcp_ses->fscache = tcp_ses->primary_server->fscache;
+#endif /* CONFIG_CIFS_FSCACHE */
 
 	/* queue echo request delayed work */
 	queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval);
diff --git a/fs/cifs/fscache.c b/fs/cifs/fscache.c
index f4da693760c1..1db3437f3b7d 100644
--- a/fs/cifs/fscache.c
+++ b/fs/cifs/fscache.c
@@ -24,6 +24,7 @@ struct cifs_server_key {
 		struct in_addr	ipv4_addr;
 		struct in6_addr	ipv6_addr;
 	};
+	__u64 conn_id;
 } __packed;
 
 /*
@@ -37,6 +38,14 @@ void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
 	struct cifs_server_key key;
 	uint16_t key_len = sizeof(key.hdr);
 
+	/*
+	 * Check if cookie was already initialized so don't reinitialize it.
+	 * In the future, as we integrate with newer fscache features,
+	 * we may want to instead add a check if cookie has changed
+	 */
+	if (server->fscache)
+		return;
+
 	memset(&key, 0, sizeof(key));
 
 	/*
@@ -62,6 +71,7 @@ void cifs_fscache_get_client_cookie(struct TCP_Server_Info *server)
 		server->fscache = NULL;
 		return;
 	}
+	key.conn_id = server->conn_id;
 
 	server->fscache =
 		fscache_acquire_cookie(cifs_fscache_netfs.primary_index,
-- 
2.32.0