From 0d5b0cf246a3227d811e7bf55d756b273408e414 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Mon, 3 Oct 2016 09:48:08 +1100 Subject: fs: update atime before I/O in generic_file_read_iter After the call to ->direct_IO the final reference to the file might have been dropped by aio_complete already, and the call to file_accessed might cause a use after free. Instead update the access time before the I/O, similar to how we update the time stamps before writes. Signed-off-by: Christoph Hellwig Reviewed-by: Dave Chinner Signed-off-by: Dave Chinner --- mm/filemap.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'mm/filemap.c') diff --git a/mm/filemap.c b/mm/filemap.c index 8a287dfc5372..2f1175e86c77 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1910,16 +1910,18 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter) if (iocb->ki_flags & IOCB_DIRECT) { struct address_space *mapping = file->f_mapping; struct inode *inode = mapping->host; + struct iov_iter data = *iter; loff_t size; size = i_size_read(inode); retval = filemap_write_and_wait_range(mapping, iocb->ki_pos, iocb->ki_pos + count - 1); - if (!retval) { - struct iov_iter data = *iter; - retval = mapping->a_ops->direct_IO(iocb, &data); - } + if (retval < 0) + goto out; + file_accessed(file); + + retval = mapping->a_ops->direct_IO(iocb, &data); if (retval > 0) { iocb->ki_pos += retval; iov_iter_advance(iter, retval); @@ -1935,10 +1937,8 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter) * DAX files, so don't bother trying. */ if (retval < 0 || !iov_iter_count(iter) || iocb->ki_pos >= size || - IS_DAX(inode)) { - file_accessed(file); + IS_DAX(inode)) goto out; - } } retval = do_generic_file_read(file, &iocb->ki_pos, iter, retval); -- cgit v1.2.3 From d3798ae8c6f3767c726403c2ca6ecc317752c9dd Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Tue, 4 Oct 2016 22:02:08 +0200 Subject: mm: filemap: don't plant shadow entries without radix tree node When the underflow checks were added to workingset_node_shadow_dec(), they triggered immediately: kernel BUG at ./include/linux/swap.h:276! invalid opcode: 0000 [#1] SMP Modules linked in: isofs usb_storage fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast ip6t_REJECT nf_reject_ipv6 soundcore wmi acpi_als pinctrl_sunrisepoint kfifo_buf tpm_tis industrialio acpi_pad pinctrl_intel tpm_tis_core tpm nfsd auth_rpcgss nfs_acl lockd grace sunrpc dm_crypt CPU: 0 PID: 20929 Comm: blkid Not tainted 4.8.0-rc8-00087-gbe67d60ba944 #1 Hardware name: System manufacturer System Product Name/Z170-K, BIOS 1803 05/06/2016 task: ffff8faa93ecd940 task.stack: ffff8faa7f478000 RIP: page_cache_tree_insert+0xf1/0x100 Call Trace: __add_to_page_cache_locked+0x12e/0x270 add_to_page_cache_lru+0x4e/0xe0 mpage_readpages+0x112/0x1d0 blkdev_readpages+0x1d/0x20 __do_page_cache_readahead+0x1ad/0x290 force_page_cache_readahead+0xaa/0x100 page_cache_sync_readahead+0x3f/0x50 generic_file_read_iter+0x5af/0x740 blkdev_read_iter+0x35/0x40 __vfs_read+0xe1/0x130 vfs_read+0x96/0x130 SyS_read+0x55/0xc0 entry_SYSCALL_64_fastpath+0x13/0x8f Code: 03 00 48 8b 5d d8 65 48 33 1c 25 28 00 00 00 44 89 e8 75 19 48 83 c4 18 5b 41 5c 41 5d 41 5e 5d c3 0f 0b 41 bd ef ff ff ff eb d7 <0f> 0b e8 88 68 ef ff 0f 1f 84 00 RIP page_cache_tree_insert+0xf1/0x100 This is a long-standing bug in the way shadow entries are accounted in the radix tree nodes. The shrinker needs to know when radix tree nodes contain only shadow entries, no pages, so node->count is split in half to count shadows in the upper bits and pages in the lower bits. Unfortunately, the radix tree implementation doesn't know of this and assumes all entries are in node->count. When there is a shadow entry directly in root->rnode and the tree is later extended, the radix tree implementation will copy that entry into the new node and and bump its node->count, i.e. increases the page count bits. Once the shadow gets removed and we subtract from the upper counter, node->count underflows and triggers the warning. Afterwards, without node->count reaching 0 again, the radix tree node is leaked. Limit shadow entries to when we have actual radix tree nodes and can count them properly. That means we lose the ability to detect refaults from files that had only the first page faulted in at eviction time. Fixes: 449dd6984d0e ("mm: keep page cache radix tree nodes in check") Signed-off-by: Johannes Weiner Reported-and-tested-by: Linus Torvalds Reviewed-by: Jan Kara Cc: Andrew Morton Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds --- include/linux/radix-tree.h | 6 +++--- lib/radix-tree.c | 14 +++----------- mm/filemap.c | 46 ++++++++++++++++++++++++++++++---------------- 3 files changed, 36 insertions(+), 30 deletions(-) (limited to 'mm/filemap.c') diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h index 4c45105dece3..52b97db93830 100644 --- a/include/linux/radix-tree.h +++ b/include/linux/radix-tree.h @@ -280,9 +280,9 @@ bool __radix_tree_delete_node(struct radix_tree_root *root, struct radix_tree_node *node); void *radix_tree_delete_item(struct radix_tree_root *, unsigned long, void *); void *radix_tree_delete(struct radix_tree_root *, unsigned long); -struct radix_tree_node *radix_tree_replace_clear_tags( - struct radix_tree_root *root, - unsigned long index, void *entry); +void radix_tree_clear_tags(struct radix_tree_root *root, + struct radix_tree_node *node, + void **slot); unsigned int radix_tree_gang_lookup(struct radix_tree_root *root, void **results, unsigned long first_index, unsigned int max_items); diff --git a/lib/radix-tree.c b/lib/radix-tree.c index 91f0727e3cad..8e6d552c40dd 100644 --- a/lib/radix-tree.c +++ b/lib/radix-tree.c @@ -1583,15 +1583,10 @@ void *radix_tree_delete(struct radix_tree_root *root, unsigned long index) } EXPORT_SYMBOL(radix_tree_delete); -struct radix_tree_node *radix_tree_replace_clear_tags( - struct radix_tree_root *root, - unsigned long index, void *entry) +void radix_tree_clear_tags(struct radix_tree_root *root, + struct radix_tree_node *node, + void **slot) { - struct radix_tree_node *node; - void **slot; - - __radix_tree_lookup(root, index, &node, &slot); - if (node) { unsigned int tag, offset = get_slot_offset(node, slot); for (tag = 0; tag < RADIX_TREE_MAX_TAGS; tag++) @@ -1600,9 +1595,6 @@ struct radix_tree_node *radix_tree_replace_clear_tags( /* Clear root node tags */ root->gfp_mask &= __GFP_BITS_MASK; } - - radix_tree_replace_slot(slot, entry); - return node; } /** diff --git a/mm/filemap.c b/mm/filemap.c index 2d0986a64f1f..96b9e9c30630 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -169,33 +169,35 @@ static int page_cache_tree_insert(struct address_space *mapping, static void page_cache_tree_delete(struct address_space *mapping, struct page *page, void *shadow) { - struct radix_tree_node *node; int i, nr = PageHuge(page) ? 1 : hpage_nr_pages(page); VM_BUG_ON_PAGE(!PageLocked(page), page); VM_BUG_ON_PAGE(PageTail(page), page); VM_BUG_ON_PAGE(nr != 1 && shadow, page); - if (shadow) { - mapping->nrexceptional += nr; - /* - * Make sure the nrexceptional update is committed before - * the nrpages update so that final truncate racing - * with reclaim does not see both counters 0 at the - * same time and miss a shadow entry. - */ - smp_wmb(); - } - mapping->nrpages -= nr; - for (i = 0; i < nr; i++) { - node = radix_tree_replace_clear_tags(&mapping->page_tree, - page->index + i, shadow); + struct radix_tree_node *node; + void **slot; + + __radix_tree_lookup(&mapping->page_tree, page->index + i, + &node, &slot); + + radix_tree_clear_tags(&mapping->page_tree, node, slot); + if (!node) { VM_BUG_ON_PAGE(nr != 1, page); - return; + /* + * We need a node to properly account shadow + * entries. Don't plant any without. XXX + */ + shadow = NULL; } + radix_tree_replace_slot(slot, shadow); + + if (!node) + break; + workingset_node_pages_dec(node); if (shadow) workingset_node_shadows_inc(node); @@ -219,6 +221,18 @@ static void page_cache_tree_delete(struct address_space *mapping, &node->private_list); } } + + if (shadow) { + mapping->nrexceptional += nr; + /* + * Make sure the nrexceptional update is committed before + * the nrpages update so that final truncate racing + * with reclaim does not see both counters 0 at the + * same time and miss a shadow entry. + */ + smp_wmb(); + } + mapping->nrpages -= nr; } /* -- cgit v1.2.3 From 3ddf40e8c31964b744ff10abb48c8e36a83ec6e7 Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Tue, 4 Oct 2016 16:58:06 +0200 Subject: mm: filemap: fix mapping->nrpages double accounting in fuse Commit 22f2ac51b6d6 ("mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()") switched replace_page_cache() from raw radix tree operations to page_cache_tree_insert() but didn't take into account that the latter function, unlike the raw radix tree op, handles mapping->nrpages. As a result, that counter is bumped for each page replacement rather than balanced out even. The mapping->nrpages counter is used to skip needless radix tree walks when invalidating, truncating, syncing inodes without pages, as well as statistics for userspace. Since the error is positive, we'll do more page cache tree walks than necessary; we won't miss a necessary one. And we'll report more buffer pages to userspace than there are. The error is limited to fuse inodes. Fixes: 22f2ac51b6d6 ("mm: workingset: fix crash in shadow node shrinker caused by replace_page_cache_page()") Signed-off-by: Johannes Weiner Cc: Andrew Morton Cc: Miklos Szeredi Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds --- mm/filemap.c | 1 - 1 file changed, 1 deletion(-) (limited to 'mm/filemap.c') diff --git a/mm/filemap.c b/mm/filemap.c index 96b9e9c30630..4bad32dd4b3b 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -633,7 +633,6 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask) __delete_from_page_cache(old, NULL); error = page_cache_tree_insert(mapping, new, NULL); BUG_ON(error); - mapping->nrpages++; /* * hugetlb pages do not participate in page cache accounting. -- cgit v1.2.3 From c4b209a426847b55c40360c1d04dc7986b55ddc7 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 7 Oct 2016 16:58:33 -0700 Subject: do_generic_file_read(): fail immediately if killed If a fatal signal has been received, fail immediately instead of trying to read more data. If wait_on_page_locked_killable() was interrupted then this page is most likely is not PageUptodate() and in this case do_generic_file_read() will fail after lock_page_killable(). See also commit ebded02788b5 ("mm: filemap: avoid unnecessary calls to lock_page when waiting for IO to complete during a read") [oleg@redhat.com: changelog addition] Link: http://lkml.kernel.org/r/63068e8e-8bee-b208-8441-a3c39a9d9eb6@sandisk.com Signed-off-by: Bart Van Assche Reviewed-by: Jan Kara Acked-by: Oleg Nesterov Acked-by: Michal Hocko Cc: Mel Gorman Cc: Hugh Dickins Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/filemap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'mm/filemap.c') diff --git a/mm/filemap.c b/mm/filemap.c index 68f1813fbdc3..1b05f75aea0f 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1721,7 +1721,9 @@ find_page: * wait_on_page_locked is used to avoid unnecessarily * serialisations and why it's safe. */ - wait_on_page_locked_killable(page); + error = wait_on_page_locked_killable(page); + if (unlikely(error)) + goto readpage_error; if (PageUptodate(page)) goto page_ok; -- cgit v1.2.3 From c2a9737f45e27d8263ff9643f994bda9bac0b944 Mon Sep 17 00:00:00 2001 From: Wei Fang Date: Fri, 7 Oct 2016 17:01:52 -0700 Subject: vfs,mm: fix a dead loop in truncate_inode_pages_range() We triggered a deadloop in truncate_inode_pages_range() on 32 bits architecture with the test case bellow: ... fd = open(); write(fd, buf, 4096); preadv64(fd, &iovec, 1, 0xffffffff000); ftruncate(fd, 0); ... Then ftruncate() will not return forever. The filesystem used in this case is ubifs, but it can be triggered on many other filesystems. When preadv64() is called with offset=0xffffffff000, a page with index=0xffffffff will be added to the radix tree of ->mapping. Then this page can be found in ->mapping with pagevec_lookup(). After that, truncate_inode_pages_range(), which is called in ftruncate(), will fall into an infinite loop: - find a page with index=0xffffffff, since index>=end, this page won't be truncated - index++, and index become 0 - the page with index=0xffffffff will be found again The data type of index is unsigned long, so index won't overflow to 0 on 64 bits architecture in this case, and the dead loop won't happen. Since truncate_inode_pages_range() is executed with holding lock of inode->i_rwsem, any operation related with this lock will be blocked, and a hung task will happen, e.g.: INFO: task truncate_test:3364 blocked for more than 120 seconds. ... call_rwsem_down_write_failed+0x17/0x30 generic_file_write_iter+0x32/0x1c0 ubifs_write_iter+0xcc/0x170 __vfs_write+0xc4/0x120 vfs_write+0xb2/0x1b0 SyS_write+0x46/0xa0 The page with index=0xffffffff added to ->mapping is useless. Fix this by checking the read position before allocating pages. Link: http://lkml.kernel.org/r/1475151010-40166-1-git-send-email-fangwei1@huawei.com Signed-off-by: Wei Fang Cc: Christoph Hellwig Cc: Dave Chinner Cc: Al Viro Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/filemap.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'mm/filemap.c') diff --git a/mm/filemap.c b/mm/filemap.c index 1b05f75aea0f..2f7b7783bd6b 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1687,6 +1687,10 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos, unsigned int prev_offset; int error = 0; + if (unlikely(*ppos >= inode->i_sb->s_maxbytes)) + return -EINVAL; + iov_iter_truncate(iter, inode->i_sb->s_maxbytes); + index = *ppos >> PAGE_SHIFT; prev_index = ra->prev_pos >> PAGE_SHIFT; prev_offset = ra->prev_pos & (PAGE_SIZE-1); -- cgit v1.2.3 From c3a690240423fc4eb8a0c3c7df025d13eadf140b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Mon, 10 Oct 2016 13:26:27 -0400 Subject: fix ITER_PIPE interaction with direct_IO by making sure we call iov_iter_advance() on original iov_iter even if direct_IO (done on its copy) has returned 0. It's a no-op for old iov_iter flavours and does the right thing (== truncation of the stuff we'd allocated, but not filled) in ITER_PIPE case. Failures (e.g. -EIO) get caught and dealt with by cleanup in generic_file_read_iter(). Signed-off-by: Al Viro --- fs/splice.c | 12 +++--------- fs/xfs/xfs_file.c | 2 +- mm/filemap.c | 2 +- 3 files changed, 5 insertions(+), 11 deletions(-) (limited to 'mm/filemap.c') diff --git a/fs/splice.c b/fs/splice.c index aa38901a4f10..153d4f3bd441 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -315,15 +315,9 @@ ssize_t generic_file_splice_read(struct file *in, loff_t *ppos, *ppos = kiocb.ki_pos; file_accessed(in); } else if (ret < 0) { - if (WARN_ON(to.idx != idx || to.iov_offset)) { - /* - * a bogus ->read_iter() has copied something and still - * returned an error instead of a short read. - */ - to.idx = idx; - to.iov_offset = 0; - iov_iter_advance(&to, 0); /* to free what was emitted */ - } + to.idx = idx; + to.iov_offset = 0; + iov_iter_advance(&to, 0); /* to free what was emitted */ /* * callers of ->splice_read() expect -EAGAIN on * "can't put anything in there", rather than -EFAULT. diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 92f16cfb81e8..26cf153e3714 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -317,7 +317,7 @@ xfs_file_dio_aio_read( data = *to; ret = __blockdev_direct_IO(iocb, inode, target->bt_bdev, &data, xfs_get_blocks_direct, NULL, NULL, 0); - if (ret > 0) { + if (ret >= 0) { iocb->ki_pos += ret; iov_iter_advance(to, ret); } diff --git a/mm/filemap.c b/mm/filemap.c index 8a287dfc5372..6b965efe215e 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -1920,7 +1920,7 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter) retval = mapping->a_ops->direct_IO(iocb, &data); } - if (retval > 0) { + if (retval >= 0) { iocb->ki_pos += retval; iov_iter_advance(iter, retval); } -- cgit v1.2.3