Commit 9687c85d authored by Rohith Surabattula's avatar Rohith Surabattula Committed by Steve French

Fix KASAN identified use-after-free issue.

[  612.157429] ==================================================================
[  612.158275] BUG: KASAN: use-after-free in process_one_work+0x90/0x9b0
[  612.158801] Read of size 8 at addr ffff88810a31ca60 by task kworker/2:9/2382

[  612.159611] CPU: 2 PID: 2382 Comm: kworker/2:9 Tainted: G
OE     5.13.0-rc2+ #98
[  612.159623] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.14.0-1.fc33 04/01/2014
[  612.159640] Workqueue:  0x0 (deferredclose)
[  612.159669] Call Trace:
[  612.159685]  dump_stack+0xbb/0x107
[  612.159711]  print_address_description.constprop.0+0x18/0x140
[  612.159733]  ? process_one_work+0x90/0x9b0
[  612.159743]  ? process_one_work+0x90/0x9b0
[  612.159754]  kasan_report.cold+0x7c/0xd8
[  612.159778]  ? lock_is_held_type+0x80/0x130
[  612.159789]  ? process_one_work+0x90/0x9b0
[  612.159812]  kasan_check_range+0x145/0x1a0
[  612.159834]  process_one_work+0x90/0x9b0
[  612.159877]  ? pwq_dec_nr_in_flight+0x110/0x110
[  612.159914]  ? spin_bug+0x90/0x90
[  612.159967]  worker_thread+0x3b6/0x6c0
[  612.160023]  ? process_one_work+0x9b0/0x9b0
[  612.160038]  kthread+0x1dc/0x200
[  612.160051]  ? kthread_create_worker_on_cpu+0xd0/0xd0
[  612.160092]  ret_from_fork+0x1f/0x30

[  612.160399] Allocated by task 2358:
[  612.160757]  kasan_save_stack+0x1b/0x40
[  612.160768]  __kasan_kmalloc+0x9b/0xd0
[  612.160778]  cifs_new_fileinfo+0xb0/0x960 [cifs]
[  612.161170]  cifs_open+0xadf/0xf20 [cifs]
[  612.161421]  do_dentry_open+0x2aa/0x6b0
[  612.161432]  path_openat+0xbd9/0xfa0
[  612.161441]  do_filp_open+0x11d/0x230
[  612.161450]  do_sys_openat2+0x115/0x240
[  612.161460]  __x64_sys_openat+0xce/0x140

When mod_delayed_work is called to modify the delay of pending work,
it might return false and queue a new work when pending work is
already scheduled or when try to grab pending work failed.

So, Increase the reference count when new work is scheduled to
avoid use-after-free.
Signed-off-by: default avatarRohith Surabattula <rohiths@microsoft.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 0ab95c25
...@@ -874,10 +874,6 @@ void smb2_deferred_work_close(struct work_struct *work) ...@@ -874,10 +874,6 @@ void smb2_deferred_work_close(struct work_struct *work)
struct cifsFileInfo, deferred.work); struct cifsFileInfo, deferred.work);
spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
if (!cfile->deferred_close_scheduled) {
spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
return;
}
cifs_del_deferred_close(cfile); cifs_del_deferred_close(cfile);
cfile->deferred_close_scheduled = false; cfile->deferred_close_scheduled = false;
spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock); spin_unlock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
...@@ -904,8 +900,13 @@ int cifs_close(struct inode *inode, struct file *file) ...@@ -904,8 +900,13 @@ int cifs_close(struct inode *inode, struct file *file)
cifs_add_deferred_close(cfile, dclose); cifs_add_deferred_close(cfile, dclose);
if (cfile->deferred_close_scheduled && if (cfile->deferred_close_scheduled &&
delayed_work_pending(&cfile->deferred)) { delayed_work_pending(&cfile->deferred)) {
mod_delayed_work(deferredclose_wq, /*
&cfile->deferred, cifs_sb->ctx->acregmax); * If there is no pending work, mod_delayed_work queues new work.
* So, Increase the ref count to avoid use-after-free.
*/
if (!mod_delayed_work(deferredclose_wq,
&cfile->deferred, cifs_sb->ctx->acregmax))
cifsFileInfo_get(cfile);
} else { } else {
/* Deferred close for files */ /* Deferred close for files */
queue_delayed_work(deferredclose_wq, queue_delayed_work(deferredclose_wq,
...@@ -4879,7 +4880,12 @@ void cifs_oplock_break(struct work_struct *work) ...@@ -4879,7 +4880,12 @@ void cifs_oplock_break(struct work_struct *work)
if (is_deferred && if (is_deferred &&
cfile->deferred_close_scheduled && cfile->deferred_close_scheduled &&
delayed_work_pending(&cfile->deferred)) { delayed_work_pending(&cfile->deferred)) {
mod_delayed_work(deferredclose_wq, &cfile->deferred, 0); /*
* If there is no pending work, mod_delayed_work queues new work.
* So, Increase the ref count to avoid use-after-free.
*/
if (!mod_delayed_work(deferredclose_wq, &cfile->deferred, 0))
cifsFileInfo_get(cfile);
} }
spin_unlock(&CIFS_I(inode)->deferred_lock); spin_unlock(&CIFS_I(inode)->deferred_lock);
_cifsFileInfo_put(cfile, false /* do not wait for ourself */, false); _cifsFileInfo_put(cfile, false /* do not wait for ourself */, false);
......
...@@ -674,6 +674,8 @@ cifs_add_pending_open(struct cifs_fid *fid, struct tcon_link *tlink, ...@@ -674,6 +674,8 @@ cifs_add_pending_open(struct cifs_fid *fid, struct tcon_link *tlink,
/* /*
* Critical section which runs after acquiring deferred_lock. * Critical section which runs after acquiring deferred_lock.
* As there is no reference count on cifs_deferred_close, pdclose
* should not be used outside deferred_lock.
*/ */
bool bool
cifs_is_deferred_close(struct cifsFileInfo *cfile, struct cifs_deferred_close **pdclose) cifs_is_deferred_close(struct cifsFileInfo *cfile, struct cifs_deferred_close **pdclose)
...@@ -752,8 +754,14 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon) ...@@ -752,8 +754,14 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon)
spin_lock(&tcon->open_file_lock); spin_lock(&tcon->open_file_lock);
list_for_each(tmp, &tcon->openFileList) { list_for_each(tmp, &tcon->openFileList) {
cfile = list_entry(tmp, struct cifsFileInfo, tlist); cfile = list_entry(tmp, struct cifsFileInfo, tlist);
if (delayed_work_pending(&cfile->deferred)) if (delayed_work_pending(&cfile->deferred)) {
mod_delayed_work(deferredclose_wq, &cfile->deferred, 0); /*
* If there is no pending work, mod_delayed_work queues new work.
* So, Increase the ref count to avoid use-after-free.
*/
if (!mod_delayed_work(deferredclose_wq, &cfile->deferred, 0))
cifsFileInfo_get(cfile);
}
} }
spin_unlock(&tcon->open_file_lock); spin_unlock(&tcon->open_file_lock);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment