On Fri, Oct 15, 2021 at 2:31 PM Christian Göttsche <cgzones@xxxxxxxxxxxxxx> wrote: > > Glibc 2.34 added an access function attribute to pthread_setspecific(3). > This leads to the following GCC warnings: > > In file included from matchpathcon.c:5: > matchpathcon.c: In function ‘matchpathcon_init_prefix’: > selinux_internal.h:38:25: error: ‘pthread_setspecific’ expecting 1 byte in a region of size 0 [-Werror=stringop-overread] > 38 | pthread_setspecific(KEY, VALUE); \ > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > matchpathcon.c:359:9: note: in expansion of macro ‘__selinux_setspecific’ > 359 | __selinux_setspecific(destructor_key, (void *)1); > | ^~~~~~~~~~~~~~~~~~~~~ > In file included from selinux_internal.h:2, > from matchpathcon.c:5: > /usr/include/pthread.h:1167:12: note: in a call to function ‘pthread_setspecific’ declared with attribute ‘access (none, 2)’ > 1167 | extern int pthread_setspecific (pthread_key_t __key, > | ^~~~~~~~~~~~~~~~~~~ > > The actual value and the validity of the passed pointer is irrelevant, > since it does not gets accessed internally by glibc and > pthread_getspecific(3) is not used. > Use a pointer to a (temporary) valid object to please GCC. > > Closes: https://github.com/SELinuxProject/selinux/issues/311 > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx> > --- > libselinux/src/matchpathcon.c | 4 +++- > libselinux/src/procattr.c | 4 +++- > libselinux/src/setrans_client.c | 4 +++- > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c > index 1e7f8890..b0ec1e8b 100644 > --- a/libselinux/src/matchpathcon.c > +++ b/libselinux/src/matchpathcon.c > @@ -352,11 +352,13 @@ static void matchpathcon_init_once(void) > > int matchpathcon_init_prefix(const char *path, const char *subset) > { > + int dummy; > + > if (!mycanoncon) > mycanoncon = default_canoncon; > > __selinux_once(once, matchpathcon_init_once); > - __selinux_setspecific(destructor_key, (void *)1); > + __selinux_setspecific(destructor_key, &dummy); > > options[SELABEL_OPT_SUBSET].type = SELABEL_OPT_SUBSET; > options[SELABEL_OPT_SUBSET].value = subset; > diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c > index 6552ee01..24cee323 100644 > --- a/libselinux/src/procattr.c > +++ b/libselinux/src/procattr.c > @@ -68,7 +68,9 @@ void __attribute__((destructor)) procattr_destructor(void) > static inline void init_thread_destructor(void) > { > if (destructor_initialized == 0) { > - __selinux_setspecific(destructor_key, (void *)1); > + int dummy; > + > + __selinux_setspecific(destructor_key, &dummy); > destructor_initialized = 1; > } > } > diff --git a/libselinux/src/setrans_client.c b/libselinux/src/setrans_client.c > index 52a8ba78..515d2d4d 100644 > --- a/libselinux/src/setrans_client.c > +++ b/libselinux/src/setrans_client.c > @@ -272,7 +272,9 @@ static inline void init_thread_destructor(void) > if (!has_setrans) > return; > if (destructor_initialized == 0) { > - __selinux_setspecific(destructor_key, (void *)1); > + int dummy; > + > + __selinux_setspecific(destructor_key, &dummy); > destructor_initialized = 1; > } > } > -- > 2.33.0 > Hello, Thanks for working on this. While this patch fixes the precise warning, I fear that future improvements on the analysis performed by compilers (or by static analysis tools) will lead to future warnings, as we are still abusing the pthread_setspecific interface after your patch: "dummy" is not initialized (so a tool could report a "use of uninitialized variable"), and is destroyed after the scope of the functions (so a tool could report a "use after free" or a "use of a stack pointer beyond its scope"). These issues can be fixed by making dummy "static char dummy;" for example. What do you think? Thanks, Nicolas
