inode.i_flags might be altered without proper synchronisation when the inode belongs to devtmpfs. blkdev_write_iter() starts writing via __generic_file_write_iter() which sets S_NOSEC bit without any synchronisation. The following stacktrace shows how to get there: 13: entry_SYSENTER_32:460 12: do_fast_syscall_32:410 11: _static_cpu_has:146 10: do_syscall_32_irqs_on:322 09: SyS_pwrite64:636 08: SYSC_pwrite64:650 07: fdput:38 06: vfs_write:560 05: __vfs_write:512 04: new_sync_write:500 03: blkdev_write_iter:1977 02: __generic_file_write_iter:2897 01: file_remove_privs:1818 00: inode_has_no_xattr:3163 If S_NOSEC is *not* set, i_rwsem is acquired around __generic_file_write_iter().
Found by LockDoc (Alexander Lochmann, Horst Schirmeier and Olaf Spinczyk) Signed-off-by: Alexander Lochmann <[email protected]> Signed-off-by: Horst Schirmeier <[email protected]> --- fs/block_dev.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/block_dev.c b/fs/block_dev.c index a80b4f0ee7c4..b4ece62e3c05 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1894,8 +1894,10 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) { struct file *file = iocb->ki_filp; struct inode *bd_inode = bdev_file_inode(file); + struct inode *inode = file_inode(file); loff_t size = i_size_read(bd_inode); struct blk_plug plug; + int locked = 0; ssize_t ret; if (bdev_read_only(I_BDEV(bd_inode))) @@ -1913,9 +1915,19 @@ ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) iov_iter_truncate(from, size - iocb->ki_pos); blk_start_plug(&plug); + /* + * Ensure excl. access to i_flags in __generic_file_write_iter(). + * Otherwise, it would race with chmod adding SUID bit. + */ + if (!IS_NOSEC(inode)) { + inode_lock(inode); + locked = 1; + } ret = __generic_file_write_iter(iocb, from); if (ret > 0) ret = generic_write_sync(iocb, ret); + if (locked) + inode_unlock(inode); blk_finish_plug(&plug); return ret; } -- 2.19.2
signature.asc
Description: OpenPGP digital signature
