SHARED MODULES BROKEN pdb: Increase version number to fix ABI
Andrew Bartlett
abartlet at samba.org
Thu Dec 11 11:25:55 MST 2014
On Thu, 2014-12-11 at 13:18 +0200, Alexander Bokovoy wrote:
> NACK to Garming's patch. The problem with it is that it doesn't solve
> the problem, it only makes it worse.
How so?
> pdb initialization code relies on the static variable representing
> list of registered modules (backends).
Indeed?
> When you turn pdb into a grouping subsystem instead of linking
> everything against a proper dynamic library, you get 'backends' static
> variable duplicated multiple times in each compilation unit that
> references it. Unfortunately, nothing will help you here to check that
> there are duplicates of 'backends' variable because it is static. We
> also have some subsystems that depend on 'pdb' subsystem and they then
> get loaded into the same process.
Given that is the very definition of a grouping library, I'm concerned,
because that implies we need to look back at grouping libraries and work
out why they are not doing that.
> As result, when static modules initialized, they operate on a
> different backends variable than when dynamic modules are loaded
> because they come through different code paths, linked from different
> subsystems.
>
> You can easily verify that there are multiple definitions of that
> variable by querying a symbol from source3/passdb/pdb_interface.c
> (where static variable is defined), like smb_register_passdb.
>
> Current state in master if I compile most of passdb modules shared as
> Andreas mentioned in his email:
> $ find bin -name '*.*o' |while read ; do usage=$(nm -a $REPLY |grep
> smb_register_passdb) ; test $? -eq 0 && printf "%46s %s\n" "$REPLY"
> "$usage"; done
> bin/default/examples/pdb/libpdb-test.so 00000000000292f5 T
> smb_register_passdb
> bin/default/examples/pdb/test_1.o U
> smb_register_passdb
> bin/default/source3/pam_smbpass/libpamsmbpass.so 000000000002fce7 T
> smb_register_passdb
> bin/default/source3/passdb/libpdb-tdbsam.so 000000000002d11a T
> smb_register_passdb
> bin/default/source3/passdb/pdb_tdb_1.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_samba_dsdb_9.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_nds_3.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_wbc_sam_7.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_ldap_3.o U
> smb_register_passdb
> bin/default/source3/passdb/pypassdb.so 0000000000034de0 T
> smb_register_passdb
> bin/default/source3/passdb/libpdb-wbc-sam.so 0000000000029afc T
> smb_register_passdb
> bin/default/source3/passdb/libpdb-smbpasswd.so 000000000002ea8f T
> smb_register_passdb
> bin/default/source3/passdb/libpdb-ldapsam.so 00000000000454f8 T
> smb_register_passdb
> bin/default/source3/passdb/pdb_interface_28.o 000000000000001f T
> smb_register_passdb
> bin/default/source3/passdb/pdb_smbpasswd_5.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_ipa_3.o U
> smb_register_passdb
> bin/default/source3/libsamba-passdb.so 0000000000028c8c T
> smb_register_passdb
> bin/modules/samba-passdb/pdb_wbc_sam.so 0000000000029afc T
> smb_register_passdb
> bin/modules/samba-passdb/pdb_smbpasswd.so 000000000002ea8f T
> smb_register_passdb
> bin/modules/samba-passdb/pdb_ldapsam.so 00000000000454f8 T
> smb_register_passdb
> bin/modules/samba-passdb/pdb_tdbsam.so 000000000002d11a T
> smb_register_passdb
> bin/modules/samba-passdb/pdb_test.so 00000000000292f5 T
> smb_register_passdb
> bin/modules/pdb-modules/pdb_wbc_sam.so 0000000000029afc T
> smb_register_passdb
> bin/modules/pdb-modules/pdb_smbpasswd.so 000000000002ea8f T
> smb_register_passdb
> bin/modules/pdb-modules/pdb_ldapsam.so 00000000000454f8 T
> smb_register_passdb
> bin/modules/pdb-modules/pdb_tdbsam.so 000000000002d11a T
> smb_register_passdb
> bin/modules/pdb/test.so 00000000000292f5 T
> smb_register_passdb
> bin/modules/pdb/ldapsam.so 00000000000454f8 T
> smb_register_passdb
> bin/modules/pdb/tdbsam.so 000000000002d11a T
> smb_register_passdb
> bin/modules/pdb/smbpasswd.so 000000000002ea8f T
> smb_register_passdb
> bin/modules/pdb/wbc_sam.so 0000000000029afc T
> smb_register_passdb
> bin/python/samba/samba3/passdb.so 0000000000034de0 T
> smb_register_passdb
> bin/shared/pam_smbpass.so 000000000002fce7 T
> smb_register_passdb
>
> As you can see, there are 24 shared modules where smb_register_passdb
> function is defined which means there are 24 places with 'backends'
> static linked list. This is calling for a disaster.
Indeed, and we saw that (looking at the behaviour, thanks for the nm
commands). What did you get with the patch on this thread BTW?
> With the attached patch I get:
> $ find bin -name '*.*o' |while read ; do usage=$(nm -a $REPLY |grep
> smb_register_passdb) ; test $? -eq 0 && printf "%46s %s\n" "$REPLY"
> "$usage"; done
> bin/default/examples/pdb/libpdb-test.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/default/examples/pdb/test_1.o U
> smb_register_passdb
> bin/default/source3/passdb/libpdb-tdbsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/default/source3/passdb/pdb_tdb_1.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_samba_dsdb_9.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_nds_3.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_wbc_sam_7.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_ldap_3.o U
> smb_register_passdb
> bin/default/source3/passdb/libpdb-wbc-sam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/default/source3/passdb/libpdb-smbpasswd.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/default/source3/passdb/libpdb-ldapsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/default/source3/passdb/pdb_interface_28.o 000000000000001f T
> smb_register_passdb
> bin/default/source3/passdb/pdb_smbpasswd_5.o U
> smb_register_passdb
> bin/default/source3/passdb/pdb_ipa_3.o U
> smb_register_passdb
> bin/default/source3/libsamba-passdb.so 0000000000028c8c T
> smb_register_passdb
> bin/modules/samba-passdb/pdb_wbc_sam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/samba-passdb/pdb_smbpasswd.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/samba-passdb/pdb_ldapsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/samba-passdb/pdb_tdbsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/samba-passdb/pdb_test.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb-modules/pdb_wbc_sam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb-modules/pdb_smbpasswd.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb-modules/pdb_ldapsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb-modules/pdb_tdbsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb/test.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb/ldapsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb/tdbsam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb/smbpasswd.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
> bin/modules/pdb/wbc_sam.so U
> smb_register_passdb@@SAMBA_PASSDB_0.2.0
>
> As you can see, there is one place where the unit with
> smb_register_passdb is defined (pdb_interface_28.o) and it is linked
> into libsamba-passdb.so which is then used by all other code.
Does your approach change the on-disk layout or affect the runtime init
code? I would have loved to just do the global rename - and if we do,
please drop the 'pdb' subsystem entirely - but when I first looked into
this back at the time the original bug was filed, it seemed that the
"subsystem" argument was being used for more than just linking rules.
Perhaps some of that is easier now we just have waf, or doesn't actually
matter?
Thanks,
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list