From a3b7c00484e1177e7eb9b047c46cac571b82442f Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 30 Sep 2014 14:50:28 +0100 Subject: CacheFiles: Handle object being killed before being set up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a cache object gets killed whilst in the process of being set up - for instance if the netfs relinquishes the cookie that the object is associated with - then the object's state machine will transit to the DROP_OBJECT state without necessarily going through the LOOKUP_OBJECT or CREATE_OBJECT states. This is a problem for CacheFiles because cachefiles_drop_object() assumes that object->dentry will be set upon reaching the DROP_OBJECT state and has an ASSERT() to that effect (see the oops below) - but object->dentry doesn't get set until the LOOKUP_OBJECT or CREATE_OBJECT states (and not always then if they fail). To fix this, just make the dentry cleanup in cachefiles_drop_object() conditional on the dentry actually being set and remove the assertion. CacheFiles: Assertion failed ------------[ cut here ]------------ kernel BUG at .../fs/cachefiles/namei.c:425! ... Workqueue: fscache_object fscache_object_work_func [fscache] ... RIP: ... cachefiles_delete_object+0xcd/0x110 [cachefiles] ... Call Trace: [] ? cachefiles_drop_object+0xff/0x130 [cachefiles] [] ? fscache_drop_object+0xd1/0x1d0 [fscache] [] ? fscache_object_work_func+0x87/0x210 [fscache] [] ? process_one_work+0x155/0x450 [] ? worker_thread+0x114/0x370 [] ? manage_workers.isra.21+0x2c0/0x2c0 [] ? kthread+0xbc/0xe0 [] ? flush_kthread_worker+0xa0/0xa0 [] ? ret_from_fork+0x7c/0xb0 [] ? flush_kthread_worker+0xa0/0xa0 Reported-by: Manuel Schölling Signed-off-by: David Howells Acked-by: Steve Dickson --- fs/cachefiles/interface.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) (limited to 'fs/cachefiles') diff --git a/fs/cachefiles/interface.c b/fs/cachefiles/interface.c index 584743d456c3..1c7293c3a93a 100644 --- a/fs/cachefiles/interface.c +++ b/fs/cachefiles/interface.c @@ -268,20 +268,27 @@ static void cachefiles_drop_object(struct fscache_object *_object) ASSERT((atomic_read(&object->usage) & 0xffff0000) != 0x6b6b0000); #endif - /* delete retired objects */ - if (test_bit(FSCACHE_OBJECT_RETIRED, &object->fscache.flags) && - _object != cache->cache.fsdef - ) { - _debug("- retire object OBJ%x", object->fscache.debug_id); - cachefiles_begin_secure(cache, &saved_cred); - cachefiles_delete_object(cache, object); - cachefiles_end_secure(cache, saved_cred); - } + /* We need to tidy the object up if we did in fact manage to open it. + * It's possible for us to get here before the object is fully + * initialised if the parent goes away or the object gets retired + * before we set it up. + */ + if (object->dentry) { + /* delete retired objects */ + if (test_bit(FSCACHE_OBJECT_RETIRED, &object->fscache.flags) && + _object != cache->cache.fsdef + ) { + _debug("- retire object OBJ%x", object->fscache.debug_id); + cachefiles_begin_secure(cache, &saved_cred); + cachefiles_delete_object(cache, object); + cachefiles_end_secure(cache, saved_cred); + } - /* close the filesystem stuff attached to the object */ - if (object->backer != object->dentry) - dput(object->backer); - object->backer = NULL; + /* close the filesystem stuff attached to the object */ + if (object->backer != object->dentry) + dput(object->backer); + object->backer = NULL; + } /* note that the object is now inactive */ if (test_bit(CACHEFILES_OBJECT_ACTIVE, &object->flags)) { -- cgit From a30efe261b5a8fb2e3cf8ea9c3aca51e0619c2cc Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 30 Sep 2014 14:50:30 +0100 Subject: CacheFiles: Fix incorrect test for in-memory object collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When CacheFiles cache objects are in use, they have in-memory representations, as defined by the cachefiles_object struct. These are kept in a tree rooted in the cache and indexed by dentry pointer (since there's a unique mapping between object index key and dentry). Collisions can occur between a representation already in the tree and a new representation being set up because it takes time to dispose of an old representation - particularly if it must be unlinked or renamed. When such a collision occurs, cachefiles_mark_object_active() is meant to check to see if the old, already-present representation is in the process of being discarded (ie. FSCACHE_OBJECT_IS_LIVE is not set on it) - and, if so, wait for the representation to be removed (ie. CACHEFILES_OBJECT_ACTIVE is then cleared). However, the test for whether the old representation is still live is checking the new object - which always will be live at this point. This leads to an oops looking like: CacheFiles: Error: Unexpected object collision object: OBJ1b354 objstate=LOOK_UP_OBJECT fl=8 wbusy=2 ev=0[0] ops=0 inp=0 exc=0 parent=ffff88053f5417c0 cookie=ffff880538f202a0 [pr=ffff8805381b7160 nd=ffff880509c6eb78 fl=27] key=[8] '2490000000000000' xobject: OBJ1a600 xobjstate=DROP_OBJECT fl=70 wbusy=2 ev=0[0] xops=0 inp=0 exc=0 xparent=ffff88053f5417c0 xcookie=ffff88050f4cbf70 [pr=ffff8805381b7160 nd= (null) fl=12] ------------[ cut here ]------------ kernel BUG at fs/cachefiles/namei.c:200! ... Workqueue: fscache_object fscache_object_work_func [fscache] ... RIP: ... cachefiles_walk_to_object+0x7ea/0x860 [cachefiles] ... Call Trace: [] ? cachefiles_lookup_object+0x58/0x100 [cachefiles] [] ? fscache_look_up_object+0xb9/0x1d0 [fscache] [] ? fscache_parent_ready+0x2d/0x80 [fscache] [] ? fscache_object_work_func+0x92/0x1f0 [fscache] [] ? process_one_work+0x16b/0x400 [] ? worker_thread+0x116/0x380 [] ? manage_workers.isra.21+0x290/0x290 [] ? kthread+0xbc/0xe0 [] ? flush_kthread_worker+0x80/0x80 [] ? ret_from_fork+0x7c/0xb0 [] ? flush_kthread_worker+0x80/0x80 Reported-by: Manuel Schölling Signed-off-by: David Howells Acked-by: Steve Dickson --- fs/cachefiles/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/cachefiles') diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index dad7d9542a24..e12f189d539b 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -189,7 +189,7 @@ try_again: /* an old object from a previous incarnation is hogging the slot - we * need to wait for it to be destroyed */ wait_for_old_object: - if (fscache_object_is_live(&object->fscache)) { + if (fscache_object_is_live(&xobject->fscache)) { pr_err("\n"); pr_err("Error: Unexpected object collision\n"); cachefiles_printk_object(object, xobject); -- cgit