Date: Wed, 31 Dec 2008 23:36:44 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: des@des.no, ru@freebsd.org Cc: yanefbsd@gmail.com, bz@freebsd.org, arch@freebsd.org Subject: Re: svn commit: r186519 - head Message-ID: <20081231.233644.1974810120.imp@bsdimp.com> In-Reply-To: <86r63p5334.fsf@ds4.des.no> References: <26259E4E-6E26-4DAE-8046-80C7C46B7CD5@gmail.com> <20081227.193308.255407637.imp@bsdimp.com> <86r63p5334.fsf@ds4.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <86r63p5334.fsf@ds4.des.no>
Dag-Erling_Sm=F8rgrav <des@des.no> writes:
: "M. Warner Losh" <imp@bsdimp.com> writes:
: > Still working on a solution that's correct and mutually acceptable =
to
: > DES. i think the ball is in my court.
: =
: No, it's a build system issue. If it's in anyone's court, it would b=
e
: ru@'s.
: =
: The problem is that we need to use different CFLAGS for .o and .So. =
The
: quick and dirty fix is to override the .c.o rule for PAM modules:
: =
: Index: /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc
: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
: --- /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc (revi=
sion 186063)
: +++ /home/des/freebsd/base/head/lib/libpam/modules/Makefile.inc (work=
ing copy)
: @@ -19,4 +19,7 @@
: LDADD+=3D -lpam
: .endif
: =
: +.c.o:
: + ${CC} ${CFLAGS} -DOPENPAM_STATIC_MODULES -c ${.IMPSRC}
: +
: .include "../Makefile.inc"
I just tried this, and it doesn't work for me. I still get the
errors. The problem, I think, is that this doesn't get to the
root-cause of the issues I'm seeing. This define doesn't affect
enough things. If we look at
contrib/openpam/include/security/openpam.h, we see:
/*
* Infrastructure for static modules using GCC linker sets.
* You are not expected to understand this.
*/
#if defined(__FreeBSD__)
# define PAM_SOEXT ".so"
#else
# undef NO_STATIC_MODULES
# define NO_STATIC_MODULES
#endif
#if defined(__GNUC__) && !defined(__PIC__) && !defined(NO_STATIC_MODULE=
S)
/* gcc, static linking */
# include <sys/cdefs.h>
# include <linker_set.h>
# define OPENPAM_STATIC_MODULES
# define PAM_EXTERN static
# define PAM_MODULE_ENTRY(name) \
static char _pam_name[] =3D name PAM_SOEXT; \
static struct pam_module _pam_module =3D { \
.path =3D _pam_name, \
.func =3D { \
[PAM_SM_AUTHENTICATE] =3D _PAM_SM_AUTHENTICATE, \
[PAM_SM_SETCRED] =3D _PAM_SM_SETCRED, \
[PAM_SM_ACCT_MGMT] =3D _PAM_SM_ACCT_MGMT, \
[PAM_SM_OPEN_SESSION] =3D _PAM_SM_OPEN_SESSION, \
[PAM_SM_CLOSE_SESSION] =3D _PAM_SM_CLOSE_SESSION, \
[PAM_SM_CHAUTHTOK] =3D _PAM_SM_CHAUTHTOK \
}, \
}; \
DATA_SET(_openpam_static_modules, _pam_module)
#else
/* normal case */
# define PAM_EXTERN
# define PAM_MODULE_ENTRY(name)
#endif
so defining OPENPAM_STATIC_MODULES doesn't affect the PAM_EXTERN
define, so I get multiple defined things because __PIC__ is always
defined for mips, both for .o and .so compilation (because MIPS always
does PIC code for ABI compliance):
../modules/pam_deny/libpam_deny.a(pam_deny.o)(.text+0x3c): In function =
`pam_sm_open_session':
: multiple definition of `pam_sm_open_session'
../modules/pam_chroot/libpam_chroot.a(pam_chroot.o)(.text+0x14): first =
defined here
../modules/pam_deny/libpam_deny.a(pam_deny.o)(.text+0x50): In function =
`pam_sm_close_session':
: multiple definition of `pam_sm_close_session'
../modules/pam_chroot/libpam_chroot.a(pam_chroot.o)(.text+0x0): first d=
efined here
../modules/pam_echo/libpam_echo.a(pam_echo.o)(.text+0x0): In function `=
pam_sm_setcred':
: multiple definition of `pam_sm_setcred'
../modules/pam_deny/libpam_deny.a(pam_deny.o)(.text+0x0): first defined=
here
<etc>
The only thing that I've seen that really works is the following
unsatisfying kludge:
Index: include/security/openpam.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- include/security/openpam.h (revision 186587)
+++ include/security/openpam.h (working copy)
@@ -308,8 +308,9 @@
/*
* Infrastructure for static modules using GCC linker sets.
* You are not expected to understand this.
+ * And it doesn't work on mips, so is disabled there.
*/
-#if defined(__FreeBSD__)
+#if defined(__FreeBSD__) && !defined(__mips__)
# define PAM_SOEXT ".so"
#else
# undef NO_STATIC_MODULES
Or the following also works, which seems less kludgy. Note, this
hasn't been run-time tested yet, just compile time. It resolves the
overloading of __PIC__ to mean 'compiling dynamic libraries' by
relying on the OPENPAM_STATIC_MODULES being defined. It also has the
advantage of not disabling this functionality on mips. It has the
very mild disadvantage of not wrapping at 80 columns too :)
Index: contrib/openpam/include/security/openpam.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- contrib/openpam/include/security/openpam.h (revision 186639)
+++ contrib/openpam/include/security/openpam.h (working copy)
@@ -316,11 +316,10 @@
# define NO_STATIC_MODULES
#endif
=
-#if defined(__GNUC__) && !defined(__PIC__) && !defined(NO_STATIC_MODUL=
ES)
+#if defined(__GNUC__) && defined(OPENPAM_STATIC_MODULES) && !defined(N=
O_STATIC_MODULES)
/* gcc, static linking */
# include <sys/cdefs.h>
# include <linker_set.h>
-# define OPENPAM_STATIC_MODULES
# define PAM_EXTERN static
# define PAM_MODULE_ENTRY(name) \
static char _pam_name[] =3D name PAM_SOEXT; \
Index: lib/libpam/modules/Makefile.inc
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- lib/libpam/modules/Makefile.inc (revision 186639)
+++ lib/libpam/modules/Makefile.inc (working copy)
@@ -19,4 +19,7 @@
LDADD+=3D -lpam
.endif
=
+.c.o:
+ ${CC} ${CFLAGS} -DOPENPAM_STATIC_MODULES -c ${.IMPSRC}
+
.include "../Makefile.inc"
btw, I suspect that if we have a #define that's really true only for
.so's, then we should use that. My quick look at the spec file didn't
show one. I think one will have to come from the build system, and
I'm a little hesitant to innovate there too much without first having
a discussion about the right way to go. Looking at your patch here,
it seems to be along the same lines that I'm thinking, but I'm not
sure I like the new variable names.
Comments?
Warner
P.S. I've moved this discussion to arch@, which appears to be our
best, high S/N mailing list these days...
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081231.233644.1974810120.imp>