ext4: fix racy use-after-free in ext4_end_io_dio()
PATCH 1/1 ext4: fix racy use-after-free in ext4_end_io_dio()
ext4_end_io_dio() queues io_end->work and then clears iocb->private; however, io_end->work completes the iocb by calling aio_complete(), which may happen before io_end->work clearing thus leading to use-after-free.
Detected and tested with slab poisoning.
---
I *think* this is the corret fix but am not too familiar with code path, so please proceed with caution.
Thank you.
fs/ext4/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ext4/inode.c +1 -1
@@ -2806,8 +2806,8 @@ out:spin_unlock_irqrestore (&ei->i_completed_io_lock, flags);/* queue the work to convert unwritten extents to written */ -queue_work (wq, &io_end->work); iocb->private =NULL ; +queue_work (wq, &io_end->work);/* XXX: probably should move into the real I/O completion handler */ inode_dio_done (inode);
Ted Ts'o
maintainerQuoted 11 lines · Tejun Heo
ext4_end_io_dio()queuesio_end->workand then clearsiocb->private; however,io_end->workcompletes the iocb by callingaio_complete(), which may happen beforeio_end->workclearing thus leading to use-after-free.Detected and tested with slab poisoning.
Signed-off-by Tejun Heo <[email protected]>Reported-by Kent Overstreet <[email protected]>Tested-by Kent Overstreet <[email protected]>
Thanks!! I've been trying to track down this bug for a while. The repro case I had ran the 12 fio's against 12 different file systems with the following configuration:
[global]
direct=1
ioengine=libaio
iodepth=1
bs=4k
ba=4k
size=128m
[create]
filename=${TESTDIR}
rw=write
... and would leave a few inodes with elevated i_ioend_counts, which means any attempt to delete those inodes or to unmount the file system owning those inodes would hang forever.
With your patch this problem goes away.
Quoted 2 lines
I *think* this is the correct fix but am not too familiar with code path, so please proceed with caution.
Looks good to me. Thanks, applied.
Quoted 1 line
Thank you.
No, thank *you*! :-)
- Ted
P.S. It would be nice to get this into xfstests, but it requires at least 10-12 (12 to repro it reliably) HDD's, and a fairly high core count machine in order to reproduce it. I played around with trying to create a reproducer that worked on a smaller number of disks and/or fio's/CPU's, but I was never able to manage it.
Heh. It took me about 2 seconds to trigger it in vm :)
One reason it triggered so fast is that my VM test setup runs everything out of ram (the disks on the host are files in a tmpfs), but the main reason we were hitting it is that bcache usually runs the bio->bi_endio function out of a workqueue, not irq context.
It also seems to only trigger when a dio write is extending a file; the same test setup run against an existing file doesn't ever cause (visible) slab corruption.
Do you think this would also explain the corruption D is seeing in vd? I haven't yet figured out a mechanism but the bug seems to fit.
Quoted 44 lines · Ted Ts'o
Quoted 11 lines · Tejun Heo
ext4_end_io_dio()queuesio_end->workand then clearsiocb->private; however,io_end->workcompletes the iocb by callingaio_complete(), which may happen beforeio_end->workclearing thus leading to use-after-free.Detected and tested with slab poisoning.
Signed-off-by Tejun Heo <[email protected]>Reported-by Kent Overstreet <[email protected]>Tested-by Kent Overstreet <[email protected]>Thanks!! I've been trying to track down this bug for a while. The repro case I had ran the 12 fio's against 12 different file systems with the following configuration:
[global]
direct=1
ioengine=libaio
iodepth=1
bs=4k
ba=4k
size=128m[create]
filename=${TESTDIR}
rw=write... and would leave a few inodes with elevated
i_ioend_counts, which means any attempt to delete those inodes or to unmount the file system owning those inodes would hang forever.With your patch this problem goes away.
Quoted 2 lines
I *think* this is the correct fix but am not too familiar with code path, so please proceed with caution.
Looks good to me. Thanks, applied.
Quoted 1 line
Thank you.
No, thank *you*! :-)
Quoted 8 lines
- Ted
P.S. It would be nice to get this into xfstests, but it requires at least 10-12 (12 to repro it reliably) HDD's, and a fairly high core count machine in order to reproduce it. I played around with trying to create a reproducer that worked on a smaller number of disks and/or fio's/CPU's, but I was never able to manage it.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html