smb: client: ensure open_cached_dir_by_dentry() only returns valid cfid

open_cached_dir_by_dentry() was exposing an invalid cached directory to
callers. The validity check outside the function was exclusively based
on cfid->time.

Add validity check before returning success and introduce
is_valid_cached_dir() helper for consistent checks across the code.

Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
Reviwed-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index b6f538a1d5af8a59926f2ad0345f7a6a0a521da6..d714b7ba99ec9bd9674f247935a5c22dba5c8a68 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -36,9 +36,8 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
                         * fully cached or it may be in the process of
                         * being deleted due to a lease break.
                         */
-                       if (!cfid->time || !cfid->has_lease) {
+                       if (!is_valid_cached_dir(cfid))
                                return NULL;
-                       }
                        kref_get(&cfid->refcount);
                        return cfid;
                }
@@ -194,7 +193,7 @@ replay_again:
         * Otherwise, it is either a new entry or laundromat worker removed it
         * from @cfids->entries.  Caller will put last reference if the latter.
         */
-       if (cfid->has_lease && cfid->time) {
+       if (is_valid_cached_dir(cfid)) {
                cfid->last_access_time = jiffies;
                spin_unlock(&cfids->cfid_list_lock);
                *ret_cfid = cfid;
@@ -233,7 +232,7 @@ replay_again:
                        list_for_each_entry(parent_cfid, &cfids->entries, entry) {
                                if (parent_cfid->dentry == dentry->d_parent) {
                                        cifs_dbg(FYI, "found a parent cached file handle\n");
-                                       if (parent_cfid->has_lease && parent_cfid->time) {
+                                       if (is_valid_cached_dir(parent_cfid)) {
                                                lease_flags
                                                        |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
                                                memcpy(pfid->parent_lease_key,
@@ -420,6 +419,8 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
        spin_lock(&cfids->cfid_list_lock);
        list_for_each_entry(cfid, &cfids->entries, entry) {
                if (dentry && cfid->dentry == dentry) {
+                       if (!is_valid_cached_dir(cfid))
+                               break;
                        cifs_dbg(FYI, "found a cached file handle by dentry\n");
                        kref_get(&cfid->refcount);
                        *ret_cfid = cfid;

diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index c98f02943311517e5e46c5afbcd7f131d19fd3d5..9210caf801645957e385001244e909a446f38273 100644 (file)
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -73,6 +73,12 @@ struct cached_fids {
 /* Module-wide directory cache accounting (defined in cifsfs.c) */
 extern atomic64_t cifs_dircache_bytes_used; /* bytes across all mounts */
 
+static inline bool
+is_valid_cached_dir(struct cached_fid *cfid)
+{
+       return cfid->time && cfid->has_lease;
+}
+
 extern struct cached_fids *init_cached_dirs(void);
 extern void free_cached_dirs(struct cached_fids *cfids);
 extern int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,

diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 5223edf6d11a5b87d799e0e70d6becffda456c42..56c59b67ecc267947e56adb8b59763886d2846e3 100644 (file)
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -322,7 +322,7 @@ retry_open:
                list_for_each_entry(parent_cfid, &tcon->cfids->entries, entry) {
                        if (parent_cfid->dentry == direntry->d_parent) {
                                cifs_dbg(FYI, "found a parent cached file handle\n");
-                               if (parent_cfid->has_lease && parent_cfid->time) {
+                               if (is_valid_cached_dir(parent_cfid)) {
                                        lease_flags
                                                |= SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET_LE;
                                        memcpy(fid->parent_lease_key,

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 7e97840805013026bfa2bab47fa19559cfc64e4b..8bb544be401e2eefd45f390cef5b73a0c2188d97 100644 (file)
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2704,7 +2704,7 @@ cifs_dentry_needs_reval(struct dentry *dentry)
                return true;
 
        if (!open_cached_dir_by_dentry(tcon, dentry->d_parent, &cfid)) {
-               if (cfid->time && cifs_i->time > cfid->time) {
+               if (cifs_i->time > cfid->time) {
                        close_cached_dir(cfid);
                        return false;
                }