| Message ID | 20221017233201.1716316-2-lsahlber@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | cifs: set rc to -ENOENT if we can not get a dentry for the cached dir | expand |
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 --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) {
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(-)