diff mbox series

cifs: set rc to -ENOENT if we can not get a dentry for the cached dir

Message ID 20221017233201.1716316-2-lsahlber@redhat.com (mailing list archive)
State New, archived
Headers show
Series cifs: set rc to -ENOENT if we can not get a dentry for the cached dir | expand

Commit Message

Ronnie Sahlberg Oct. 17, 2022, 11:32 p.m. UTC
We already set rc to this return code further down in the function but
we can set it earlier in order to suppress a smash warning.

Also fix a false positive for Coverity. The reason this is a false positive is
that this happens during umount after all files and directories have been closed
but mosetting on ->on_list to suppress the warning.

Reported-by: Dan carpenter <dan.carpenter@oracle.com>
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1525256 ("Concurrent data access violations")
Fixes: a350d6e73f5e ("cifs: enable caching of directories for which a lease is held")
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cached_dir.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Steve French Oct. 17, 2022, 11:52 p.m. UTC | #1
Lightly updated to deal with merge conflict with "cifs: use
LIST_HEAD() and list_move() to simplify code" and merged into
cifs-2.6.git for-next pending review and testing

See attached update patch.

On Mon, Oct 17, 2022 at 6:32 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> We already set rc to this return code further down in the function but
> we can set it earlier in order to suppress a smash warning.
>
> Also fix a false positive for Coverity. The reason this is a false positive is
> that this happens during umount after all files and directories have been closed
> but mosetting on ->on_list to suppress the warning.
>
> Reported-by: Dan carpenter <dan.carpenter@oracle.com>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1525256 ("Concurrent data access violations")
> Fixes: a350d6e73f5e ("cifs: enable caching of directories for which a lease is held")
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/cached_dir.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/cached_dir.c b/fs/cifs/cached_dir.c
> index fe88b67c863f..ffc924296e59 100644
> --- a/fs/cifs/cached_dir.c
> +++ b/fs/cifs/cached_dir.c
> @@ -253,8 +253,10 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
>                 dentry = dget(cifs_sb->root);
>         else {
>                 dentry = path_to_dentry(cifs_sb, path);
> -               if (IS_ERR(dentry))
> +               if (IS_ERR(dentry)) {
> +                       rc = -ENOENT;
>                         goto oshr_free;
> +               }
>         }
>         cfid->dentry = dentry;
>         cfid->tcon = tcon;
> @@ -387,13 +389,13 @@ void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
>                 list_add(&cfid->entry, &entry);
>                 cfids->num_entries--;
>                 cfid->is_open = false;
> +               cfid->on_list = false;
>                 /* To prevent race with smb2_cached_lease_break() */
>                 kref_get(&cfid->refcount);
>         }
>         spin_unlock(&cfids->cfid_list_lock);
>
>         list_for_each_entry_safe(cfid, q, &entry, entry) {
> -               cfid->on_list = false;
>                 list_del(&cfid->entry);
>                 cancel_work_sync(&cfid->lease_break);
>                 if (cfid->has_lease) {
> --
> 2.35.3
>
diff mbox series

Patch

diff --git a/fs/cifs/cached_dir.c b/fs/cifs/cached_dir.c
index fe88b67c863f..ffc924296e59 100644
--- a/fs/cifs/cached_dir.c
+++ b/fs/cifs/cached_dir.c
@@ -253,8 +253,10 @@  int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
 		dentry = dget(cifs_sb->root);
 	else {
 		dentry = path_to_dentry(cifs_sb, path);
-		if (IS_ERR(dentry))
+		if (IS_ERR(dentry)) {
+			rc = -ENOENT;
 			goto oshr_free;
+		}
 	}
 	cfid->dentry = dentry;
 	cfid->tcon = tcon;
@@ -387,13 +389,13 @@  void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
 		list_add(&cfid->entry, &entry);
 		cfids->num_entries--;
 		cfid->is_open = false;
+		cfid->on_list = false;
 		/* To prevent race with smb2_cached_lease_break() */
 		kref_get(&cfid->refcount);
 	}
 	spin_unlock(&cfids->cfid_list_lock);
 
 	list_for_each_entry_safe(cfid, q, &entry, entry) {
-		cfid->on_list = false;
 		list_del(&cfid->entry);
 		cancel_work_sync(&cfid->lease_break);
 		if (cfid->has_lease) {