| Message ID | 20190311150041.373-1-plautrba@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libselinux: Do not define gettid() if glibc >= 2.30 is used | expand |
On 3/11/19 11:00 AM, Petr Lautrbach wrote: > Since version 2.30 glibc implements gettid() system call wrapper, see > https://sourceware.org/bugzilla/show_bug.cgi?id=6399 > > Fixes: > cc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND -c -o procattr.o procattr.c > procattr.c:28:14: error: static declaration of ‘gettid’ follows non-static declaration > 28 | static pid_t gettid(void) > | ^~~~~~ > In file included from /usr/include/unistd.h:1170, > from procattr.c:2: > /usr/include/bits/unistd_ext.h:34:16: note: previous declaration of ‘gettid’ was here > 34 | extern __pid_t gettid (void) __THROW; > | ^~~~~~ > > Signed-off-by: Petr Lautrbach <plautrba@redhat.com> I assume the glibc change will break a lot of software out there that assumed that "never" meant "never" and rolled their own gettid() wrapper. Would have been nice if they had at least added some #define to indicate that it is now provided for easy testing rather than needing to test minor version number. Anyway, regardless, Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > libselinux/src/procattr.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c > index 48dd8aff..c6799ef2 100644 > --- a/libselinux/src/procattr.c > +++ b/libselinux/src/procattr.c > @@ -22,8 +22,19 @@ static pthread_key_t destructor_key; > static int destructor_key_initialized = 0; > static __thread char destructor_initialized; > > -#ifndef __BIONIC__ > -/* Bionic declares this in unistd.h and has a definition for it */ > +/* Bionic and glibc >= 2.30 declare gettid() system call wrapper in unistd.h and > + * has a definition for it */ > +#ifdef __BIONIC__ > + #define OVERRIDE_GETTID 0 > +#elif !defined(__GLIBC_PREREQ) > + #define OVERRIDE_GETTID 1 > +#elif !__GLIBC_PREREQ(2,30) > + #define OVERRIDE_GETTID 1 > +#else > + #define OVERRIDE_GETTID 0 > +#endif > + > +#if OVERRIDE_GETTID > static pid_t gettid(void) > { > return syscall(__NR_gettid); >
Stephen Smalley <sds@tycho.nsa.gov> writes: > On 3/11/19 11:00 AM, Petr Lautrbach wrote: >> Since version 2.30 glibc implements gettid() system call >> wrapper, see >> https://sourceware.org/bugzilla/show_bug.cgi?id=6399 >> >> Fixes: >> cc -O2 -g -pipe -Wall -Werror=format-security >> -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions >> -fstack-protector-strong -grecord-gcc-switches >> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 >> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 >> -mtune=generic -fasynchronous-unwind-tables >> -fstack-clash-protection -fcf-protection -I../include >> -D_GNU_SOURCE -DNO_ANDROID_BACKEND -c -o procattr.o >> procattr.c >> procattr.c:28:14: error: static declaration of ‘gettid’ follows >> non-static declaration >> 28 | static pid_t gettid(void) >> | ^~~~~~ >> In file included from /usr/include/unistd.h:1170, >> from procattr.c:2: >> /usr/include/bits/unistd_ext.h:34:16: note: previous >> declaration of ‘gettid’ was here >> 34 | extern __pid_t gettid (void) __THROW; >> | ^~~~~~ >> >> Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > I assume the glibc change will break a lot of software out there > that assumed > that "never" meant "never" and rolled their own gettid() > wrapper. Would have > been nice if they had at least added some #define to indicate > that it is now > provided for easy testing rather than needing to test minor > version number. > Anyway, regardless, > I asked for an advice from glibc Fedora maintainer [1] as the current version is Fedora Rawhide is still 2.29 but it already ships gettid wrapped linked as gettid@@GLIBC_2.30. The response was that I should rename libselinux gettid() to something else in order to prevent such conflict. It would mean that libselinux won't depend on glibc implementation. But I decided to test the minor version and use the new glibc wrapper when it's possible. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1685594 > Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > >> --- >> libselinux/src/procattr.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/libselinux/src/procattr.c >> b/libselinux/src/procattr.c >> index 48dd8aff..c6799ef2 100644 >> --- a/libselinux/src/procattr.c >> +++ b/libselinux/src/procattr.c >> @@ -22,8 +22,19 @@ static pthread_key_t destructor_key; >> static int destructor_key_initialized = 0; >> static __thread char destructor_initialized; >> -#ifndef __BIONIC__ >> -/* Bionic declares this in unistd.h and has a definition for >> it */ >> +/* Bionic and glibc >= 2.30 declare gettid() system call >> wrapper in unistd.h and >> + * has a definition for it */ >> +#ifdef __BIONIC__ >> + #define OVERRIDE_GETTID 0 >> +#elif !defined(__GLIBC_PREREQ) >> + #define OVERRIDE_GETTID 1 >> +#elif !__GLIBC_PREREQ(2,30) >> + #define OVERRIDE_GETTID 1 >> +#else >> + #define OVERRIDE_GETTID 0 >> +#endif >> + >> +#if OVERRIDE_GETTID >> static pid_t gettid(void) >> { >> return syscall(__NR_gettid); >>
On 3/11/19 12:42 PM, Petr Lautrbach wrote: > > Stephen Smalley <sds@tycho.nsa.gov> writes: > >> On 3/11/19 11:00 AM, Petr Lautrbach wrote: >>> Since version 2.30 glibc implements gettid() system call wrapper, see >>> https://sourceware.org/bugzilla/show_bug.cgi?id=6399 >>> >>> Fixes: >>> cc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 >>> -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong >>> -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 >>> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic >>> -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection >>> -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND -c -o procattr.o >>> procattr.c >>> procattr.c:28:14: error: static declaration of ‘gettid’ follows >>> non-static declaration >>> 28 | static pid_t gettid(void) >>> | ^~~~~~ >>> In file included from /usr/include/unistd.h:1170, >>> from procattr.c:2: >>> /usr/include/bits/unistd_ext.h:34:16: note: previous declaration of >>> ‘gettid’ was here >>> 34 | extern __pid_t gettid (void) __THROW; >>> | ^~~~~~ >>> >>> Signed-off-by: Petr Lautrbach <plautrba@redhat.com> >> >> I assume the glibc change will break a lot of software out there that >> assumed >> that "never" meant "never" and rolled their own gettid() wrapper. >> Would have >> been nice if they had at least added some #define to indicate that it >> is now >> provided for easy testing rather than needing to test minor version >> number. >> Anyway, regardless, >> > > I asked for an advice from glibc Fedora maintainer [1] as the current > version is Fedora Rawhide is still 2.29 but it already ships gettid > wrapped linked as gettid@@GLIBC_2.30. The response was that I should > rename libselinux gettid() to something else in order to prevent such > conflict. It would mean that libselinux won't depend on glibc > implementation. But I decided to test the minor version and use > the new glibc wrapper when it's possible. Yes, your way seems better - no point in providing our own wrapper once glibc has it. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1685594 > >> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> >> >>> --- >>> libselinux/src/procattr.c | 15 +++++++++++++-- >>> 1 file changed, 13 insertions(+), 2 deletions(-) >>> >>> diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c >>> index 48dd8aff..c6799ef2 100644 >>> --- a/libselinux/src/procattr.c >>> +++ b/libselinux/src/procattr.c >>> @@ -22,8 +22,19 @@ static pthread_key_t destructor_key; >>> static int destructor_key_initialized = 0; >>> static __thread char destructor_initialized; >>> -#ifndef __BIONIC__ >>> -/* Bionic declares this in unistd.h and has a definition for it */ >>> +/* Bionic and glibc >= 2.30 declare gettid() system call wrapper in >>> unistd.h and >>> + * has a definition for it */ >>> +#ifdef __BIONIC__ >>> + #define OVERRIDE_GETTID 0 >>> +#elif !defined(__GLIBC_PREREQ) >>> + #define OVERRIDE_GETTID 1 >>> +#elif !__GLIBC_PREREQ(2,30) >>> + #define OVERRIDE_GETTID 1 >>> +#else >>> + #define OVERRIDE_GETTID 0 >>> +#endif >>> + >>> +#if OVERRIDE_GETTID >>> static pid_t gettid(void) >>> { >>> return syscall(__NR_gettid); >>> >
On 3/11/19 12:23 PM, Stephen Smalley wrote: > On 3/11/19 11:00 AM, Petr Lautrbach wrote: >> Since version 2.30 glibc implements gettid() system call wrapper, see >> https://sourceware.org/bugzilla/show_bug.cgi?id=6399 >> >> Fixes: >> cc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 >> -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong >> -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 >> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic >> -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection >> -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND -c -o procattr.o >> procattr.c >> procattr.c:28:14: error: static declaration of ‘gettid’ follows >> non-static declaration >> 28 | static pid_t gettid(void) >> | ^~~~~~ >> In file included from /usr/include/unistd.h:1170, >> from procattr.c:2: >> /usr/include/bits/unistd_ext.h:34:16: note: previous declaration of >> ‘gettid’ was here >> 34 | extern __pid_t gettid (void) __THROW; >> | ^~~~~~ >> >> Signed-off-by: Petr Lautrbach <plautrba@redhat.com> > > I assume the glibc change will break a lot of software out there that > assumed that "never" meant "never" and rolled their own gettid() > wrapper. Would have been nice if they had at least added some #define > to indicate that it is now provided for easy testing rather than needing > to test minor version number. Anyway, regardless, > > Acked-by: Stephen Smalley <sds@tycho.nsa.gov> Thanks, applied. > >> --- >> libselinux/src/procattr.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c >> index 48dd8aff..c6799ef2 100644 >> --- a/libselinux/src/procattr.c >> +++ b/libselinux/src/procattr.c >> @@ -22,8 +22,19 @@ static pthread_key_t destructor_key; >> static int destructor_key_initialized = 0; >> static __thread char destructor_initialized; >> -#ifndef __BIONIC__ >> -/* Bionic declares this in unistd.h and has a definition for it */ >> +/* Bionic and glibc >= 2.30 declare gettid() system call wrapper in >> unistd.h and >> + * has a definition for it */ >> +#ifdef __BIONIC__ >> + #define OVERRIDE_GETTID 0 >> +#elif !defined(__GLIBC_PREREQ) >> + #define OVERRIDE_GETTID 1 >> +#elif !__GLIBC_PREREQ(2,30) >> + #define OVERRIDE_GETTID 1 >> +#else >> + #define OVERRIDE_GETTID 0 >> +#endif >> + >> +#if OVERRIDE_GETTID >> static pid_t gettid(void) >> { >> return syscall(__NR_gettid); >> >
diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c index 48dd8aff..c6799ef2 100644 --- a/libselinux/src/procattr.c +++ b/libselinux/src/procattr.c @@ -22,8 +22,19 @@ static pthread_key_t destructor_key; static int destructor_key_initialized = 0; static __thread char destructor_initialized; -#ifndef __BIONIC__ -/* Bionic declares this in unistd.h and has a definition for it */ +/* Bionic and glibc >= 2.30 declare gettid() system call wrapper in unistd.h and + * has a definition for it */ +#ifdef __BIONIC__ + #define OVERRIDE_GETTID 0 +#elif !defined(__GLIBC_PREREQ) + #define OVERRIDE_GETTID 1 +#elif !__GLIBC_PREREQ(2,30) + #define OVERRIDE_GETTID 1 +#else + #define OVERRIDE_GETTID 0 +#endif + +#if OVERRIDE_GETTID static pid_t gettid(void) { return syscall(__NR_gettid);
Since version 2.30 glibc implements gettid() system call wrapper, see https://sourceware.org/bugzilla/show_bug.cgi?id=6399 Fixes: cc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND -c -o procattr.o procattr.c procattr.c:28:14: error: static declaration of ‘gettid’ follows non-static declaration 28 | static pid_t gettid(void) | ^~~~~~ In file included from /usr/include/unistd.h:1170, from procattr.c:2: /usr/include/bits/unistd_ext.h:34:16: note: previous declaration of ‘gettid’ was here 34 | extern __pid_t gettid (void) __THROW; | ^~~~~~ Signed-off-by: Petr Lautrbach <plautrba@redhat.com> --- libselinux/src/procattr.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)