Commit c5f9d9db authored May 04, 2020 by David Howells

cachefiles: Fix corruption of the return value in cachefiles_read_or_alloc_pages()



The patch which changed cachefiles from calling ->bmap() to using the
bmap() wrapper overwrote the running return value with the result of
calling bmap().  This causes an assertion failure elsewhere in the code.

Fix this by using ret2 rather than ret to hold the return value.

The oops looks like:

	kernel BUG at fs/nfs/fscache.c:468!
	...
	RIP: 0010:__nfs_readpages_from_fscache+0x18b/0x190 [nfs]
	...
	Call Trace:
	 nfs_readpages+0xbf/0x1c0 [nfs]
	 ? __alloc_pages_nodemask+0x16c/0x320
	 read_pages+0x67/0x1a0
	 __do_page_cache_readahead+0x1cf/0x1f0
	 ondemand_readahead+0x172/0x2b0
	 page_cache_async_readahead+0xaa/0xe0
	 generic_file_buffered_read+0x852/0xd50
	 ? mem_cgroup_commit_charge+0x6e/0x140
	 ? nfs4_have_delegation+0x19/0x30 [nfsv4]
	 generic_file_read_iter+0x100/0x140
	 ? nfs_revalidate_mapping+0x176/0x2b0 [nfs]
	 nfs_file_read+0x6d/0xc0 [nfs]
	 new_sync_read+0x11a/0x1c0
	 __vfs_read+0x29/0x40
	 vfs_read+0x8e/0x140
	 ksys_read+0x61/0xd0
	 __x64_sys_read+0x1a/0x20
	 do_syscall_64+0x60/0x1e0
	 entry_SYSCALL_64_after_hwframe+0x44/0xa9
	RIP: 0033:0x7f5d148267e0

Fixes: 10d83e11 ("cachefiles: drop direct usage of ->bmap method.")
Reported-by: David Wysochanski <dwysocha@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: David Wysochanski <dwysocha@redhat.com>
cc: Carlos Maiolino <cmaiolino@redhat.com>

parent 6a8b55ed

fs/cachefiles/rdwr.c

+5 −5

Original line number	Diff line number	Diff line
		@@ -398,7 +398,7 @@ int cachefiles_read_or_alloc_page(struct fscache_retrieval *op,
		struct inode *inode;
		sector_t block;
		unsigned shift;
		int ret;
		int ret, ret2;

		object = container_of(op->op.object,
		struct cachefiles_object, fscache);
		@@ -430,8 +430,8 @@ int cachefiles_read_or_alloc_page(struct fscache_retrieval *op,
		block = page->index;
		block <<= shift;

		ret = bmap(inode, &block);
		ASSERT(ret < 0);
		ret2 = bmap(inode, &block);
		ASSERT(ret2 == 0);

		_debug("%llx -> %llx",
		(unsigned long long) (page->index << shift),
		@@ -739,8 +739,8 @@ int cachefiles_read_or_alloc_pages(struct fscache_retrieval *op,
		block = page->index;
		block <<= shift;

		ret = bmap(inode, &block);
		ASSERT(!ret);
		ret2 = bmap(inode, &block);
		ASSERT(ret2 == 0);

		_debug("%llx -> %llx",
		(unsigned long long) (page->index << shift),

Admin message