On Thu, 6 Feb 2014 11:45:10 +1100 NeilBrown <neilb@xxxxxxx> wrote:
> On Wed, 05 Feb 2014 16:09:51 +0000 David Howells <dhowells@xxxxxxxxxx> wrote:
>
> > NeilBrown <neilb@xxxxxxx> wrote:
> >
> > > fscache_alloc_object() calls fscache_attach_object() and if that fails it
> > > calls cache->ops->put_object(), which is cachefiles_put_object() which calls
> > > fscache_object_destroy(), which calls fscache_objlist_remove() which calls
> > > rb_erase() to remove the object from fscache_object_list.
> > >
> > > However the object isn't added to fscache_object_list except on the success
> > > path of fscache_attach_object() where it calls fscache_objlist_add().
> > >
> > > So if fscache_attach_object() fails, then the object wasn't added to the
> > > objlist rbtree, but is removed from it. This causes a crash.
> > >
> > > One approach would be to rb_init_node(&object->objlist_link) in
> > > fscache_object_init, and the test !RB_EMPTY_NODE(&obj->objlist_link) before
> > > calling rb_erase().
> > >
> > > Does that make sense? Is the suggested fix best or is there a better way?
> >
> > Sounds very reasonable. Something like the attached patch?
>
> Yes, that looks just right. Thanks.
> I'll let you know when the customer confirms it fixes their problem.
I now have definite confirmation that this fixes the customer's problem.
So
Tested-by: (a customer of) NeilBrown <neilb@xxxxxxx>
Thanks,
NeilBrown
>
> NeilBrown
>
> >
> > David
> > ---
> > commit fe02fb3ec10932ce07406b1581e28326181fc9d8
> > Author: David Howells <dhowells@xxxxxxxxxx>
> > Date: Wed Feb 5 15:14:40 2014 +0000
> >
> > FS-Cache: Handle removal of unadded object to the fscache_object_list rb tree
> >
> > When FS-Cache allocates an object, the following sequence of events can occur:
> >
> > -->fscache_alloc_object()
> > -->cachefiles_alloc_object() [via cache->ops->alloc_object]
> > <--[returns new object]
> > -->fscache_attach_object()
> > <--[failed]
> > -->cachefiles_put_object() [via cache->ops->put_object]
> > -->fscache_object_destroy()
> > -->fscache_objlist_remove()
> > -->rb_erase() to remove the object from fscache_object_list.
> >
> > resulting in a crash in the rbtree code.
> >
> > The problem is that the object is only added to fscache_object_list on the
> > success path of fscache_attach_object() where it calls fscache_objlist_add().
> >
> > So if fscache_attach_object() fails, the object won't have been added to the
> > objlist rbtree. We do, however, unconditionally try to remove the object from
> > the tree.
> >
> > Thanks to NeilBrown for finding this and suggesting this solution.
> >
> > Reported-by: NeilBrown <neilb@xxxxxxx>
> > Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> >
> > diff --git a/fs/fscache/object-list.c b/fs/fscache/object-list.c
> > index e1959efad64f..b5ebc2d7d80d 100644
> > --- a/fs/fscache/object-list.c
> > +++ b/fs/fscache/object-list.c
> > @@ -50,6 +50,8 @@ void fscache_objlist_add(struct fscache_object *obj)
> > struct fscache_object *xobj;
> > struct rb_node **p = &fscache_object_list.rb_node, *parent = NULL;
> >
> > + ASSERT(RB_EMPTY_NODE(&obj->objlist_link));
> > +
> > write_lock(&fscache_object_list_lock);
> >
> > while (*p) {
> > @@ -75,6 +77,9 @@ void fscache_objlist_add(struct fscache_object *obj)
> > */
> > void fscache_objlist_remove(struct fscache_object *obj)
> > {
> > + if (RB_EMPTY_NODE(&obj->objlist_link))
> > + return;
> > +
> > write_lock(&fscache_object_list_lock);
> >
> > BUG_ON(RB_EMPTY_ROOT(&fscache_object_list));
> > diff --git a/fs/fscache/object.c b/fs/fscache/object.c
> > index 53d35c504240..d3b4539f1651 100644
> > --- a/fs/fscache/object.c
> > +++ b/fs/fscache/object.c
> > @@ -314,6 +314,9 @@ void fscache_object_init(struct fscache_object *object,
> > object->cache = cache;
> > object->cookie = cookie;
> > object->parent = NULL;
> > +#ifdef CONFIG_FSCACHE_OBJECT_LIST
> > + RB_CLEAR_NODE(&object->objlist_link);
> > +#endif
> >
> > object->oob_event_mask = 0;
> > for (t = object->oob_table; t->events; t++)
>
Attachment:
signature.asc
Description: PGP signature
-- Linux-cachefs mailing list Linux-cachefs@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/linux-cachefs
