[Gluster-devel] libgfapi usage issues: overwrites 'THIS' and use after free

[Jeff Darcy]

> 1. When libgfapi is used by the GlusterFS internal xlators, 'THIS'
> gets overwritten: Eg: when snapview-server creates a new fs instance
> for every snap that is created.  Currently any libgfapi calls inside
> the xlator overwrites the THIS value to contain glfs-master(gfapi
> xlator).  Hence after the api exits, any further code in the parent
> xlator referring to THIS will refer to the glfs-master(gfapi xlator).

> Proposed solutions: - Store and restore THIS in every API exposed by
> libgfapi, patch for the same can be found at
> http://review.gluster.org/#/c/9797/ - Other solution suggested by
> Niels was to not have internal xlators calling libgfapi and move the
> core functionality to libglusterfs. But even with this the nested
> mount/ctx issue can still exist.

Saving/restoring THIS seems like the right thing to do.  However, I
think we should also consider before deciding on a solution.

(1) Is using GFAPI from a translator really a valid use case?  When we
first hit this exact problem in NSR, such "mixing and matching" seemed
like a mistake.  It still does.  The problem with THIS is almost
certainly not going to the the last case of GFAPI interacting badly with
translators.  Long term, we might want to consider changing USS to use
an extended single graph instead of embedding a GFAPI graph inside the
normal one.

(2) Can we avoid having GFAPI manipulate THIS at all?  THIS is a bit of
a hack, always replaceable by using "this" (lowercase) and passing it as
a parameter where it doesn't already exist.  Go ahead, look at what THIS
actually is/does, and try to argue that it's a good idea to use it
everywhere.  Relying on it in various parts of the code is (again)
likely to bite us repeatedly, so perhaps we should be eliminating such
usage instead of enabling it.

Again: for *now*, 9797 seems like the right thing to do.  I just don't
want to stop there.  We should also be trying to reduce some of the
technical debt that we've accrued in this area.