[patch] prevent slab corruption by fixing race codition in cifs
Shirish Pargaonkar <[email protected]>
| Newsgroups | gmane.linux.file-systems.cifs |
|---|---|
| Message-ID | <[email protected]> |
This patch prevents a slab corruption like this. During heavy stress, it is possible that cifs_close will free up cifsFileInfo while due to delayed writes, wrtPending of that cifsFileInfo gets updated (decremented), cifsFileInfo either freed or freed and allocated to another process. Slab corruption: start=ffff8101e28e3818, len=256 Redzone: 0x5a2cf071/0x5a2cf071. Last user: [<ffffffff88276ec4>](cifs_close+0x224/0x2c2 [cifs]) 060: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6a 6b 6b 6b Prev obj: start=ffff8101e28e3700, len=256 Redzone: 0x170fc2a5/0x170fc2a5. Last user: [<ffffffff882784fa>](cifs_open+0x348/0x6d9 [cifs]) 000: b8 d3 44 e2 01 81 ff ff a0 e2 c7 e1 01 81 ff ff 010: 68 f5 fa dc 01 81 ff ff 10 47 d4 dd 01 81 ff ff Next obj: start=ffff8101e28e3930, len=256 Redzone: 0x170fc2a5/0x170fc2a5. Last user: [<ffffffff882784fa>](cifs_open+0x348/0x6d9 [cifs]) 000: 18 b8 2a e3 01 81 ff ff b8 a3 5a e0 01 81 ff ff 010: 68 f5 36 65 02 81 ff ff 40 99 a1 dc 01 81 ff ff _______________________________________________ linux-cifs-client mailing list [email protected] https://lists.samba.org/mailman/listinfo/linux-cifs-client
cifs.slabcorruption.patch
(application/octet-stream, 4.1 KB)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 6084d63..9ad3258 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -349,6 +349,7 @@ struct cifsFileInfo {
struct mutex lock_mutex;
struct list_head llist; /* list of byte range locks we have. */
bool closePend:1; /* file is marked to close */
+ bool closed:1; /* file is closed */
bool invalidHandle:1; /* file closed via session abend */
bool messageMode:1; /* for pipes: message vs byte mode */
atomic_t wrtPending; /* handle in use - defer close */
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c34b7f8..f1ae25c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -698,7 +698,10 @@ int cifs_close(struct inode *inode, struct file *file)
msleep(timeout);
timeout *= 8;
}
- kfree(file->private_data);
+ if (atomic_read(&pSMBFile->wrtPending) == 0)
+ kfree(file->private_data);
+ else
+ pSMBFile->closed = true;
file->private_data = NULL;
} else
rc = -EBADF;
@@ -1293,7 +1296,10 @@ refind_writable:
else { /* start over in case this was deleted */
/* since the list could be modified */
read_lock(&GlobalSMBSeslock);
- atomic_dec(&open_file->wrtPending);
+ if (atomic_dec_and_test(
+ &open_file->wrtPending)
+ && open_file->closed)
+ kfree(open_file);
goto refind_writable;
}
}
@@ -1309,10 +1315,12 @@ refind_writable:
read_lock(&GlobalSMBSeslock);
/* can not use this handle, no write
pending on this one after all */
- atomic_dec(&open_file->wrtPending);
-
- if (open_file->closePend) /* list could have changed */
+ if (atomic_dec_and_test(&open_file->wrtPending)
+ && open_file->closed) {
+ kfree(open_file);
goto refind_writable;
+ } else if (open_file->closePend) /* list could */
+ goto refind_writable; /* have changed */
/* 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
@@ -1373,7 +1381,9 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to)
if (open_file) {
bytes_written = cifs_write(open_file->pfile, write_data,
to-from, &offset);
- atomic_dec(&open_file->wrtPending);
+ if (atomic_dec_and_test(&open_file->wrtPending)
+ && open_file->closed)
+ kfree(open_file);
/* Does mm or vfs already set times? */
inode->i_atime = inode->i_mtime = current_fs_time(inode->i_sb);
if ((bytes_written > 0) && (offset))
@@ -1562,7 +1572,9 @@ retry:
bytes_to_write, offset,
&bytes_written, iov, n_iov,
long_op);
- atomic_dec(&open_file->wrtPending);
+ if (atomic_dec_and_test(&open_file->wrtPending)
+ && open_file->closed)
+ kfree(open_file);
cifs_update_eof(cifsi, offset, bytes_written);
if (rc || bytes_written < bytes_to_write) {
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 82d8383..3b4c27d 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -799,8 +799,11 @@ set_via_filehandle:
if (open_file == NULL)
CIFSSMBClose(xid, pTcon, netfid);
- else
- atomic_dec(&open_file->wrtPending);
+ else {
+ if (atomic_dec_and_test(&open_file->wrtPending)
+ && open_file->closed)
+ kfree(open_file);
+ }
out:
return rc;
}
@@ -1635,7 +1638,9 @@ cifs_set_file_size(struct inode *inode, struct iattr *attrs,
__u32 npid = open_file->pid;
rc = CIFSSMBSetFileSize(xid, pTcon, attrs->ia_size, nfid,
npid, false);
- atomic_dec(&open_file->wrtPending);
+ if (atomic_dec_and_test(&open_file->wrtPending)
+ && open_file->closed)
+ kfree(open_file);
cFYI(1, ("SetFSize for attrs rc = %d", rc));
if ((rc == -EINVAL) || (rc == -EOPNOTSUPP)) {
unsigned int bytes_written;
@@ -1790,7 +1795,9 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
u16 nfid = open_file->netfid;
u32 npid = open_file->pid;
rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid);
- atomic_dec(&open_file->wrtPending);
+ if (atomic_dec_and_test(&open_file->wrtPending)
+ && open_file->closed)
+ kfree(open_file);
} else {
rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,
cifs_sb->local_nls,