[Gluster-devel] Mark all the xlator fops 'static '

[Kaleb KEITHLEY]

On 07/12/2016 03:00 AM, Jiffin Tony Thottan wrote:
> 
> 
> On 31/07/15 19:29, Kaleb S. KEITHLEY wrote:
>> On 07/30/2015 05:16 PM, Niels de Vos wrote:
>>> On Thu, Jul 30, 2015 at 08:27:15PM +0530, Soumya Koduri wrote:
>>>> Hi,
>>>>
>>>> With the applications using and loading different libraries, the
>>>> function
>>>> symbols with the same name may get resolved incorrectly depending on
>>>> the
>>>> order in which those libraries get dynamically loaded.
>>>>
>>>> Recently we have seen an issue with 'snapview-client' xlator lookup
>>>> fop -
>>>> 'svc_lookup' which matched with one of the routines provided by
>>>> libntirpc,
>>>> used by NFS-Ganesha. More details are in [1], [2].
>>> Indeed, the problem seems to be caused in an execution flow like this:
>>>
>>> 1. nfs-ganesha main binary starts
>>> 2. the dynamic linker loads libntirpc (and others)
>>> 3. the dynamic linker retrieves symbols from the libntirpc (and others)
>>> 4. 'svc_lookup' is amoung the symbols added to a lookup table (or such)
>>> 5. during execution, ganesha loads plugins with dlopen()
>>> 6. the fsalgluster.so plugin is linked against libgfapi and gfapi gets
>>>     loaded
>>> 7. libgfapi retrieves the .vol file and loads the xlators, including
>>>     snapview-client
>>> 8. snapview-client provices a 'svc_lookup' symbol, same name as
>>>     libntirpc provides, complete different functionality
>> So far so good. But I would have expected the compiler to have populated
>> the function pointers in snapview-client's fops table at compile time;
>> the dynamic loader should not have been needed to resolve
>> snapview-client's svc_lookup, because it was (should have been) already
>> resolved at compile time.
>>
>> And in fact it is, but, there are semantics for global (.globl) symbols
>> and run-time linkage that are biting us.
>>
>>
> 
> Hi all,
> 
> I am hitting a similar type of collision on ganesha 2.4.  In ganesha
> 2.4, we introduced stackable
> mdcache at top of every FSAL. The lookup(mdc_lookup) function has
> similar signature to gluster
> mdcache lookup fop. In my case ganesha always pick up mdc_lookup from
> its layer not from
> gfapi graph. When I disabled md-cache it worked perfectly. As Soumya
> suggested before do we
> need to change every xlator fop to static?
>

The xlator fops are already effectively static, at least in 3.8 and later.

Starting in 3.8 the xlators are linked with an export map that only
exposes init(), fini(), fops, cbks, options, notify(), mem_acct_init(),
reconfigure(), and dumpops. (A few xlators export other symbols too.)

If this is biting us in 3.7 then we need to make the mdcache fops static.

This isn't C++, so all it takes for a symbol name collision is for the
functions to have the same name, i.e. mdc_lookup() in this case. :-/

--

Kaleb