[PATCH] fs: fix use-after-free in __fput() when a chardev is removed but a file is still open

From: Vladis Dronov
Date: Mon Nov 25 2019 - 07:54:12 EST


In a case when a chardev file (like /dev/ptp0) is open but an underlying
device is removed, closing this file leads to a use-after-free. This
reproduces easily in a KVM virtual machine:

# cat openptp0.c
int main() { ... fp = fopen("/dev/ptp0", "r"); ... sleep(10); }

# uname -r
5.4.0-219d5433
# cat /proc/cmdline
... slub_debug=FZP
# modprobe ptp_kvm
# ./openptp0 &
[1] 670
opened /dev/ptp0, sleeping 10s...
# rmmod ptp_kvm
# ls /dev/ptp*
ls: cannot access '/dev/ptp*': No such file or directory
# ...woken up
[ 102.375849] general protection fault: 0000 [#1] SMP
[ 102.377372] CPU: 1 PID: 670 Comm: openptp0 Not tainted 5.4.0-219d5433 #1
[ 102.379163] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), ...
[ 102.381129] RIP: 0010:module_put.part.0+0x7/0x80
[ 102.383019] RSP: 0018:ffff9ba440687e00 EFLAGS: 00010202
[ 102.383451] RAX: 0000000000002000 RBX: 6b6b6b6b6b6b6b6b RCX: ffff91e736800ad0
[ 102.384030] RDX: ffffcf6408bc2808 RSI: 0000000000000247 RDI: 6b6b6b6b6b6b6b6b
[ 102.386032] ... ^^^ a slub poison
[ 102.389866] Call Trace:
[ 102.390086] __fput+0x21f/0x240
[ 102.390363] task_work_run+0x79/0x90
[ 102.390671] do_exit+0x2c9/0xad0
[ 102.390931] ? vfs_write+0x16a/0x190
[ 102.391241] do_group_exit+0x35/0x90
[ 102.391549] __x64_sys_exit_group+0xf/0x10
[ 102.391898] do_syscall_64+0x3d/0x110
[ 102.392240] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 102.392695] RIP: 0033:0x7f0fa7016246
[ 102.396615] ...
[ 102.397225] Modules linked in: [last unloaded: ptp_kvm]
[ 102.410323] Fixing recursive fault but reboot is needed!

This happens in:

static void __fput(struct file *file)
{ ...
if (file->f_op->release)
file->f_op->release(inode, file); <<< cdev is kfree'd here
if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL &&
!(mode & FMODE_PATH))) {
cdev_put(inode->i_cdev); <<< cdev fields are accessed here

because of:

__fput()
posix_clock_release()
kref_put(&clk->kref, delete_clock) <<< the last reference
delete_clock()
delete_ptp_clock()
kfree(ptp) <<< cdev is embedded in ptp
cdev_put
module_put(p->owner) <<< *p is kfree'd

The fix is to call cdev_put() before file->f_op->release(). This fix the
class of bugs when a chardev device is removed when its file is open, for
example:

# lspci
00:09.0 System peripheral: Intel Corporation 6300ESB Watchdog Timer
# ./openwdog0 &
[1] 672
opened /dev/watchdog0, sleeping 10s...
# echo 1 > /sys/devices/pci0000:00/0000:00:09.0/remove
# ls /dev/watch*
ls: cannot access '/dev/watch*': No such file or directory
# ...woken up
[ 63.500271] general protection fault: 0000 [#1] SMP
[ 63.501757] CPU: 1 PID: 672 Comm: openwdog0 Not tainted 5.4.0-219d5433 #4
[ 63.503605] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), ...
[ 63.507064] RIP: 0010:module_put.part.0+0x7/0x80
[ 63.513841] RSP: 0018:ffffb96b00667e00 EFLAGS: 00010202
[ 63.515376] RAX: 0000000000002000 RBX: 6b6b6b6b6b6b6b6b RCX: 0000000000150013
[ 63.517478] RDX: 0000000000000246 RSI: 0000000000000000 RDI: 6b6b6b6b6b6b6b6b

Analyzed-by: Stephen Johnston <sjohnsto@xxxxxxxxxx>
Analyzed-by: Vern Lovejoy <vlovejoy@xxxxxxxxxx>
Signed-off-by: Vladis Dronov <vdronov@xxxxxxxxxx>
---
fs/file_table.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index 30d55c9a1744..21ba35024950 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -276,12 +276,12 @@ static void __fput(struct file *file)
if (file->f_op->fasync)
file->f_op->fasync(-1, file, 0);
}
- if (file->f_op->release)
- file->f_op->release(inode, file);
if (unlikely(S_ISCHR(inode->i_mode) && inode->i_cdev != NULL &&
!(mode & FMODE_PATH))) {
cdev_put(inode->i_cdev);
}
+ if (file->f_op->release)
+ file->f_op->release(inode, file);
fops_put(file->f_op);
put_pid(file->f_owner.pid);
if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
--
2.20.1