Skip to contentSkip to conversation

Patch "FS-Cache: Handle a write to the page immediately beyond the EOF marker" has been added to the 4.3-stable tree

PATCH [email protected] 1 participant 1 message Feb 14, 2016

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

FS-Cache: Handle a write to the page immediately beyond the EOF marker

to the 4.3-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
fs-cache-handle-a-write-to-the-page-immediately-beyond-the-eof-marker.patch
and it can be found in the queue-4.3 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 102f4d900c9c8f5ed89ae4746d493fe3ebd7ba64 Mon Sep 17 00:00:00 2001

From: David Howells <[email protected]>
Date: Wed, 4 Nov 2015 15:20:42 +0000
Subject: FS-Cache: Handle a write to the page immediately beyond the EOF marker

From: David Howells <[email protected]>

commit 102f4d900c9c8f5ed89ae4746d493fe3ebd7ba64 upstream.

Handle a write being requested to the page immediately beyond the EOF marker on a cache object. Currently this gets an assertion failure in CacheFiles because the EOF marker is used there to encode information about a partial page at the EOF - which could lead to an unknown blank spot in the file if we extend the file over it.

The problem is actually in fscache where we check the index of the page being written against store_limit. store_limit is set to the number of pages that we're allowed to store by fscache_set_store_limit() - which means it's one more than the index of the last page we're allowed to store. The problem is that we permit writing to a page with an index _equal_ to the store limit - when we should reject that case.

Whilst we're at it, change the triggered assertion in CacheFiles to just return -ENOBUFS instead.

The assertion failure looks something like this:

CacheFiles: Assertion failed
1000 < 7b1 is false

------------[ cut here ]------------
kernel BUG at fs/cachefiles/rdwr.c:962!
...
RIP: 0010:[<ffffffffa02c9e83>]  [<ffffffffa02c9e83>] cachefiles_write_page+0x273/0x2d0 [cachefiles]
---
 fs/cachefiles/rdwr.c |   67 ++++++++++++++++++++++++++++-----------------------
 fs/fscache/page.c    |    2 -
 2 files changed, 38 insertions(+), 31 deletions(-)
fs/cachefiles/rdwr.c +37 -30
@@ -905,6 +905,15 @@ int cachefiles_write_page(struct fscache
 	cache = container_of(object->fscache.cache,
 			     struct cachefiles_cache, cache);
 
+	pos = (loff_t)page->index << PAGE_SHIFT;
+
+	/* We mustn't write more data than we have, so we have to beware of a
+	 * partial page at EOF.
+	 */
+	eof = object->fscache.store_limit_l;
+	if (pos >= eof)
+		goto error;
+
 	/* write the page to the backing filesystem and let it store it in its
 	 * own time */
 	path.mnt = cache->mnt;
@@ -912,40 +921,38 @@ int cachefiles_write_page(struct fscache
 	file = dentry_open(&path, O_RDWR | O_LARGEFILE, cache->cache_cred);
 	if (IS_ERR(file)) {
 		ret = PTR_ERR(file);
-	} else {
-		pos = (loff_t) page->index << PAGE_SHIFT;
-
-		/* we mustn't write more data than we have, so we have
-		 * to beware of a partial page at EOF */
-		eof = object->fscache.store_limit_l;
-		len = PAGE_SIZE;
-		if (eof & ~PAGE_MASK) {
-			ASSERTCMP(pos, <, eof);
-			if (eof - pos < PAGE_SIZE) {
-				_debug("cut short %llx to %llx",
-				       pos, eof);
-				len = eof - pos;
-				ASSERTCMP(pos + len, ==, eof);
-			}
-		}
-
-		data = kmap(page);
-		ret = __kernel_write(file, data, len, &pos);
-		kunmap(page);
-		if (ret != len)
-			ret = -EIO;
-		fput(file);
+		goto error_2;
 	}
 
-	if (ret < 0) {
-		if (ret == -EIO)
-			cachefiles_io_error_obj(
-				object, "Write page to backing file failed");
-		ret = -ENOBUFS;
+	len = PAGE_SIZE;
+	if (eof & ~PAGE_MASK) {
+		if (eof - pos < PAGE_SIZE) {
+			_debug("cut short %llx to %llx",
+			       pos, eof);
+			len = eof - pos;
+			ASSERTCMP(pos + len, ==, eof);
+		}
 	}
 
-	_leave(" = %d", ret);
-	return ret;
+	data = kmap(page);
+	ret = __kernel_write(file, data, len, &pos);
+	kunmap(page);
+	fput(file);
+	if (ret != len)
+		goto error_eio;
+
+	_leave(" = 0");
+	return 0;
+
+error_eio:
+	ret = -EIO;
+error_2:
+	if (ret == -EIO)
+		cachefiles_io_error_obj(object,
+					"Write page to backing file failed");
+error:
+	_leave(" = -ENOBUFS [%d]", ret);
+	return -ENOBUFS;
 }
 
 /*
fs/fscache/page.c +1 -1
@@ -816,7 +816,7 @@ static void fscache_write_op(struct fsca
 		goto superseded;
 	page = results[0];
 	_debug("gang %d [%lx]", n, page->index);
-	if (page->index > op->store_limit) {
+	if (page->index >= op->store_limit) {
 		fscache_stat(&fscache_n_store_pages_over_limit);
 		goto superseded;
 	}

Patches currently in stable-queue which might be from [email protected] are

queue-4.3/crypto-fix-test-vector-for-rsa.patch queue-4.3/fs-cache-increase-reference-of-parent-after-registering-netfs-success.patch queue-4.3/hrtimer-handle-remaining-time-proper-for-time_low_res.patch queue-4.3/fs-cache-add-missing-initialization-of-ret-in-cachefiles_write_page.patch queue-4.3/fs-cache-handle-a-write-to-the-page-immediately-beyond-the-eof-marker.patch queue-4.3/fs-cache-don-t-override-netfs-s-primary_index-if-registering-failed.patch