[PATCH 2/2] smb: client: make lease state changes compliant with the protocol spec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Meetakshi Setiya <msetiya@xxxxxxxxxxxxx>

MS-SMB2 section 3.2.5.7.5 specifies that client must evaluate
delta_epoch to compare the old and new epoch values. This delta_epoch
takes care of lease epoch wraparounds (e.g. when the server resets
the epoch from 65535 to 0). Currently, we just check if the old epoch
is numerically less than the new epoch, which can cause problems when
the server resets its epoch counter from 65535 to 0 - like causing
the client (with current epoch > 0) to not change its lease state.
This patch uses delta_epoch based comparisons while comparing lease
epochs in smb3_downgrade_oplock and smb3_set_oplock_level.

Also, in the current code for smb3_set_oplock_level, the client
changes the lease state for a file without comparing the epoch. This
patch adds the delta_epoch comparision before updating the lease
state, so that when the change in epoch is negative, the new lease
state is invalid too. This can protect the client from having an
inconsistent lease state because of a stale lease state change
response.

This patch also adds additional validations to check if the lease
state change is valid or not, before going through
smb3_set_oplock_level.

Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Meetakshi Setiya <msetiya@xxxxxxxxxxxxx>
Reviewed-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
---
 fs/smb/client/cifsglob.h |  6 +++
 fs/smb/client/smb2ops.c  | 95 +++++++++++++++++++++++++++++++---------
 2 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 2c1b0438fe7d..4417fa46885f 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1558,6 +1558,12 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
 #define CIFS_CACHE_HANDLE(cinode) (cinode->oplock & CIFS_CACHE_HANDLE_FLG)
 #define CIFS_CACHE_WRITE(cinode) ((cinode->oplock & CIFS_CACHE_WRITE_FLG) || (CIFS_SB(cinode->netfs.inode.i_sb)->mnt_cifs_flags & CIFS_MOUNT_RW_CACHE))
 
+#define IS_SAME_EPOCH(new, cur) ((__u16)new == (__u16)cur)
+#define IS_NEWER_EPOCH(new, cur) (((short)((__u16)new - (__u16)cur) <= (short)32767) && ((__u16)new != (__u16)cur))
+
+bool validate_lease_state_change(__u32 old_state, __u32 new_state,
+				__u16 old_epoch, __u16 new_epoch);
+
 /*
  * One of these for each file inode
  */
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index ec36bed54b0b..6e0ce114fc08 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3922,7 +3922,7 @@ smb3_downgrade_oplock(struct TCP_Server_Info *server,
 	__u16 old_epoch = cinode->epoch;
 	unsigned int new_state;
 
-	if (epoch > old_epoch) {
+	if (IS_NEWER_EPOCH(epoch, old_epoch)) {
 		smb21_set_oplock_level(cinode, oplock, 0, NULL);
 		cinode->epoch = epoch;
 	}
@@ -3998,39 +3998,92 @@ smb21_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock,
 		 &cinode->netfs.inode);
 }
 
+/* helper function to ascertain that the incoming lease state is valid */
+bool
+validate_lease_state_change(__u32 old_state, __u32 new_state,
+				__u16 old_epoch, __u16 new_epoch)
+{
+	if (new_state == 0)
+		return true;
+
+	if (old_state == CIFS_CACHE_RH_FLG && new_state == CIFS_CACHE_READ_FLG)
+		return false;
+
+	if (old_state == CIFS_CACHE_RHW_FLG) {
+		if (new_state == CIFS_CACHE_READ_FLG || new_state == CIFS_CACHE_RH_FLG)
+			return false;
+	}
+
+	// lease state changes should not be possible without a valid epoch change
+	if (old_state != new_state) {
+		if (IS_SAME_EPOCH(new_epoch, old_epoch))
+			return false;
+	} else {
+		if ((old_state & new_state) == CIFS_CACHE_RHW_FLG) {
+			if (!IS_SAME_EPOCH(new_epoch, old_epoch))
+				return false;
+		}
+	}
+
+	return true;
+}
+
 static void
 smb3_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock,
 		      __u16 epoch, bool *purge_cache)
 {
 	unsigned int old_oplock = cinode->oplock;
+	unsigned int new_oplock = oplock;
+
+	if (!validate_lease_state_change(cinode->oplock, oplock, cinode->epoch, epoch)) {
+		cifs_dbg(FYI, "Invalid lease state change on inode %p\n", &cinode->netfs.inode);
+		return;
+	}
 
-	smb21_set_oplock_level(cinode, oplock, epoch, purge_cache);
+	/* if the epoch returned by the server is older than the current one,
+	 * the new lease state is stale.
+	 * In this case, just retain the existing lease level.
+	 */
+	if (IS_NEWER_EPOCH(cinode->epoch, epoch)) {
+		cifs_dbg(FYI,
+			 "Stale lease epoch received for inode %p, ignoring state change\n",
+			 &cinode->netfs.inode);
+		return;
+	}
 
-	if (purge_cache) {
+	if (purge_cache && old_oplock != 0) {
 		*purge_cache = false;
-		if (old_oplock == CIFS_CACHE_READ_FLG) {
-			if (cinode->oplock == CIFS_CACHE_READ_FLG &&
-			    (epoch - cinode->epoch > 0))
-				*purge_cache = true;
-			else if (cinode->oplock == CIFS_CACHE_RH_FLG &&
-				 (epoch - cinode->epoch > 1))
-				*purge_cache = true;
-			else if (cinode->oplock == CIFS_CACHE_RHW_FLG &&
-				 (epoch - cinode->epoch > 1))
-				*purge_cache = true;
-			else if (cinode->oplock == 0 &&
-				 (epoch - cinode->epoch > 0))
+
+		/* case 1: lease state remained the same,
+		 * - if epoch change is 0, no action
+		 * - if epoch change is > 0, purge cache
+		 */
+		if (old_oplock == new_oplock) {
+			if (IS_NEWER_EPOCH(epoch, cinode->epoch))
 				*purge_cache = true;
-		} else if (old_oplock == CIFS_CACHE_RH_FLG) {
-			if (cinode->oplock == CIFS_CACHE_RH_FLG &&
-			    (epoch - cinode->epoch > 0))
+		}
+
+		/* case 2: lease state upgraded,
+		 * - if epoch change is 1, upgrade
+		 * - if epoch change is > 1, upgrade and purge cache
+		 * we do not handle lease upgrades, so just purging the cache is ok.
+		 */
+		else if (old_oplock == (new_oplock & old_oplock)) {
+			if (IS_NEWER_EPOCH(epoch-1, cinode->epoch))
 				*purge_cache = true;
-			else if (cinode->oplock == CIFS_CACHE_RHW_FLG &&
-				 (epoch - cinode->epoch > 1))
+		}
+
+		/* case 3: lease state downgraded,
+		 * - if epoch change > 0, purge cache
+		 */
+		else {
+			if (IS_NEWER_EPOCH(epoch, cinode->epoch))
 				*purge_cache = true;
 		}
-		cinode->epoch = epoch;
 	}
+
+	smb21_set_oplock_level(cinode, new_oplock, epoch, purge_cache);
+	cinode->epoch = epoch;
 }
 
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
-- 
2.46.0.46.g406f326d27





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux