Skip to content

Commit

Permalink
cifs: fix oops while traversing open file list (try #4)
Browse files Browse the repository at this point in the history
While traversing the linked list of open file handles, if the identfied
file handle is invalid, a reopen is attempted and if it fails, we
resume traversing where we stopped and cifs can oops while accessing
invalid next element, for list might have changed.

So mark the invalid file handle and attempt reopen if no
valid file handle is found in rest of the list.
If reopen fails, move the invalid file handle to the end of the list
and start traversing the list again from the begining.
Repeat this four times before giving up and returning an error if
file reopen keeps failing.

Cc: <[email protected]>
Signed-off-by: Shirish Pargaonkar <[email protected]>
Reviewed-by: Jeff Layton <[email protected]>
Signed-off-by: Steve French <[email protected]>
  • Loading branch information
shirishpargaonkar authored and piastry committed May 23, 2012
1 parent ea4b574 commit 2c0c2a0
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 24 deletions.
1 change: 1 addition & 0 deletions fs/cifs/cifsglob.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

#define CIFS_MIN_RCV_POOL 4

#define MAX_REOPEN_ATT 5 /* these many maximum attempts to reopen a file */
/*
* default attribute cache timeout (jiffies)
*/
Expand Down
57 changes: 33 additions & 24 deletions fs/cifs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -1565,10 +1565,11 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode,
struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,
bool fsuid_only)
{
struct cifsFileInfo *open_file;
struct cifsFileInfo *open_file, *inv_file = NULL;
struct cifs_sb_info *cifs_sb;
bool any_available = false;
int rc;
unsigned int refind = 0;

/* Having a null inode here (because mapping->host was set to zero by
the VFS or MM) should not happen but we had reports of on oops (due to
Expand All @@ -1588,48 +1589,56 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode,

spin_lock(&cifs_file_list_lock);
refind_writable:
if (refind > MAX_REOPEN_ATT) {
spin_unlock(&cifs_file_list_lock);
return NULL;
}
list_for_each_entry(open_file, &cifs_inode->openFileList, flist) {
if (!any_available && open_file->pid != current->tgid)
continue;
if (fsuid_only && open_file->uid != current_fsuid())
continue;
if (OPEN_FMODE(open_file->f_flags) & FMODE_WRITE) {
cifsFileInfo_get(open_file);

if (!open_file->invalidHandle) {
/* found a good writable file */
cifsFileInfo_get(open_file);
spin_unlock(&cifs_file_list_lock);
return open_file;
} else {
if (!inv_file)
inv_file = open_file;
}

spin_unlock(&cifs_file_list_lock);

/* Had to unlock since following call can block */
rc = cifs_reopen_file(open_file, false);
if (!rc)
return open_file;

/* if it fails, try another handle if possible */
cFYI(1, "wp failed on reopen file");
cifsFileInfo_put(open_file);

spin_lock(&cifs_file_list_lock);

/* else we simply continue to the next entry. Thus
we do not loop on reopen errors. If we
can not reopen the file, for example if we
reconnected to a server with another client
racing to delete or lock the file we would not
make progress if we restarted before the beginning
of the loop here. */
}
}
/* couldn't find useable FH with same pid, try any available */
if (!any_available) {
any_available = true;
goto refind_writable;
}

if (inv_file) {
any_available = false;
cifsFileInfo_get(inv_file);
}

spin_unlock(&cifs_file_list_lock);

if (inv_file) {
rc = cifs_reopen_file(inv_file, false);
if (!rc)
return inv_file;
else {
spin_lock(&cifs_file_list_lock);
list_move_tail(&inv_file->flist,
&cifs_inode->openFileList);
spin_unlock(&cifs_file_list_lock);
cifsFileInfo_put(inv_file);
spin_lock(&cifs_file_list_lock);
++refind;
goto refind_writable;
}
}

return NULL;
}

Expand Down

0 comments on commit 2c0c2a0

Please sign in to comment.