libcom_err: Use sem_post/sem_init to prevent race conditions

SuSE has been carrying a patch for a long time to prevent a largely
theoretical race condition if a multi-threaded application adds and
removes error tables in multiple threads.  Unfortunately SuSE's
approach breaks compatibility by forcing applications to link and
compile with the -pthread option; using pthread mutexes has
historically been problematic.

This commit fixes things in a more portable way by using
sem_post/sem_wait instead, which is an older interface that doesn't
require the pthreads library.  Linux happens to implement
sem_post/sem_init using futexes, and -lrt ends up pulling in
-lpthread, but the advantage of using POSIX semaphores is that
applications don't have to be built using -pthread, unlike the use of
pthread mutexes.

The add_error_table() and remove_error_table() interfaces are the
preferred interfaces and locking protection have been added to only
these interfaces.  I have not added locking protection to the
generated initialize_xxx_error_table and initialize_xxx_error_table_r
interfaces, to avoid adding symbol dependencies that would cause a
library to fail to work when linking against older com_err libraries
that do not export et_list_lock() and et_list_unlock().  Threaded
applications shouldn't be using these interfaces in any case.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
bitmap-optimize
Theodore Ts'o 2008-09-12 10:15:26 -04:00
parent 3218dc9db8
commit d7f45af802
7 changed files with 398 additions and 13 deletions

View File

@ -71,7 +71,7 @@ MKINSTALLDIRS = @MKINSTALLDIRS@
#
LIB = $(top_builddir)/lib
LIBSS = $(LIB)/libss@LIB_EXT@ @PRIVATE_LIBS_CMT@ @DLOPEN_LIB@
LIBCOM_ERR = $(LIB)/libcom_err@LIB_EXT@
LIBCOM_ERR = $(LIB)/libcom_err@LIB_EXT@ @PRIVATE_LIBS_CMT@ @SEM_INIT_LIB@
LIBE2P = $(LIB)/libe2p@LIB_EXT@
LIBEXT2FS = $(LIB)/libext2fs@LIB_EXT@
LIBUUID = $(LIB)/libuuid@LIB_EXT@ @SOCKET_LIB@
@ -82,7 +82,7 @@ DEPLIBUUID = $(LIB)/libuuid@LIB_EXT@
DEPLIBBLKID = $(LIB)/libblkid@LIB_EXT@ @PRIVATE_LIBS_CMT@ $(LIBUUID)
STATIC_LIBSS = $(LIB)/libss@STATIC_LIB_EXT@ @DLOPEN_LIB@
STATIC_LIBCOM_ERR = $(LIB)/libcom_err@STATIC_LIB_EXT@
STATIC_LIBCOM_ERR = $(LIB)/libcom_err@STATIC_LIB_EXT@ @SEM_INIT_LIB@
STATIC_LIBE2P = $(LIB)/libe2p@STATIC_LIB_EXT@
STATIC_LIBEXT2FS = $(LIB)/libext2fs@STATIC_LIB_EXT@
STATIC_LIBUUID = $(LIB)/libuuid@STATIC_LIB_EXT@ @SOCKET_LIB@

303
configure vendored
View File

@ -751,6 +751,7 @@ SIZEOF_LONG
SIZEOF_LONG_LONG
SOCKET_LIB
DLOPEN_LIB
SEM_INIT_LIB
UNI_DIFF_OPTS
LINUX_CMT
CYGWIN_CMT
@ -11646,7 +11647,8 @@ fi
for ac_header in dirent.h errno.h getopt.h malloc.h mntent.h paths.h setjmp.h signal.h stdarg.h stdint.h stdlib.h termios.h termio.h unistd.h utime.h linux/fd.h linux/major.h net/if_dl.h netinet/in.h sys/disklabel.h sys/file.h sys/ioctl.h sys/mkdev.h sys/mman.h sys/prctl.h sys/queue.h sys/resource.h sys/select.h sys/socket.h sys/sockio.h sys/stat.h sys/syscall.h sys/sysmacros.h sys/time.h sys/types.h sys/un.h sys/wait.h
for ac_header in dirent.h errno.h getopt.h malloc.h mntent.h paths.h semaphore.h setjmp.h signal.h stdarg.h stdint.h stdlib.h termios.h termio.h unistd.h utime.h linux/fd.h linux/major.h net/if_dl.h netinet/in.h sys/disklabel.h sys/file.h sys/ioctl.h sys/mkdev.h sys/mman.h sys/prctl.h sys/queue.h sys/resource.h sys/select.h sys/socket.h sys/sockio.h sys/stat.h sys/syscall.h sys/sysmacros.h sys/time.h sys/types.h sys/un.h sys/wait.h
do
as_ac_Header=`echo "ac_cv_header_$ac_header" | $as_tr_sh`
if { as_var=$as_ac_Header; eval "test \"\${$as_var+set}\" = set"; }; then
@ -14972,6 +14974,302 @@ if test $ac_cv_have_optreset = yes; then
_ACEOF
fi
SEM_INIT_LIB=''
{ echo "$as_me:$LINENO: checking for sem_init" >&5
echo $ECHO_N "checking for sem_init... $ECHO_C" >&6; }
if test "${ac_cv_func_sem_init+set}" = set; then
echo $ECHO_N "(cached) $ECHO_C" >&6
else
cat >conftest.$ac_ext <<_ACEOF
/* confdefs.h. */
_ACEOF
cat confdefs.h >>conftest.$ac_ext
cat >>conftest.$ac_ext <<_ACEOF
/* end confdefs.h. */
/* Define sem_init to an innocuous variant, in case <limits.h> declares sem_init.
For example, HP-UX 11i <limits.h> declares gettimeofday. */
#define sem_init innocuous_sem_init
/* System header to define __stub macros and hopefully few prototypes,
which can conflict with char sem_init (); below.
Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
<limits.h> exists even on freestanding compilers. */
#ifdef __STDC__
# include <limits.h>
#else
# include <assert.h>
#endif
#undef sem_init
/* Override any GCC internal prototype to avoid an error.
Use char because int might match the return type of a GCC
builtin and then its argument prototype would still apply. */
#ifdef __cplusplus
extern "C"
#endif
char sem_init ();
/* The GNU C library defines this for functions which it implements
to always fail with ENOSYS. Some functions are actually named
something starting with __ and the normal name is an alias. */
#if defined __stub_sem_init || defined __stub___sem_init
choke me
#endif
int
main ()
{
return sem_init ();
;
return 0;
}
_ACEOF
rm -f conftest.$ac_objext conftest$ac_exeext
if { (ac_try="$ac_link"
case "(($ac_try" in
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
*) ac_try_echo=$ac_try;;
esac
eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
(eval "$ac_link") 2>conftest.er1
ac_status=$?
grep -v '^ *+' conftest.er1 >conftest.err
rm -f conftest.er1
cat conftest.err >&5
echo "$as_me:$LINENO: \$? = $ac_status" >&5
(exit $ac_status); } && {
test -z "$ac_c_werror_flag" ||
test ! -s conftest.err
} && test -s conftest$ac_exeext &&
$as_test_x conftest$ac_exeext; then
ac_cv_func_sem_init=yes
else
echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
ac_cv_func_sem_init=no
fi
rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
conftest$ac_exeext conftest.$ac_ext
fi
{ echo "$as_me:$LINENO: result: $ac_cv_func_sem_init" >&5
echo "${ECHO_T}$ac_cv_func_sem_init" >&6; }
if test $ac_cv_func_sem_init = yes; then
:
else
{ echo "$as_me:$LINENO: checking for sem_init in -lpthread" >&5
echo $ECHO_N "checking for sem_init in -lpthread... $ECHO_C" >&6; }
if test "${ac_cv_lib_pthread_sem_init+set}" = set; then
echo $ECHO_N "(cached) $ECHO_C" >&6
else
ac_check_lib_save_LIBS=$LIBS
LIBS="-lpthread $LIBS"
cat >conftest.$ac_ext <<_ACEOF
/* confdefs.h. */
_ACEOF
cat confdefs.h >>conftest.$ac_ext
cat >>conftest.$ac_ext <<_ACEOF
/* end confdefs.h. */
/* Override any GCC internal prototype to avoid an error.
Use char because int might match the return type of a GCC
builtin and then its argument prototype would still apply. */
#ifdef __cplusplus
extern "C"
#endif
char sem_init ();
int
main ()
{
return sem_init ();
;
return 0;
}
_ACEOF
rm -f conftest.$ac_objext conftest$ac_exeext
if { (ac_try="$ac_link"
case "(($ac_try" in
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
*) ac_try_echo=$ac_try;;
esac
eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
(eval "$ac_link") 2>conftest.er1
ac_status=$?
grep -v '^ *+' conftest.er1 >conftest.err
rm -f conftest.er1
cat conftest.err >&5
echo "$as_me:$LINENO: \$? = $ac_status" >&5
(exit $ac_status); } && {
test -z "$ac_c_werror_flag" ||
test ! -s conftest.err
} && test -s conftest$ac_exeext &&
$as_test_x conftest$ac_exeext; then
ac_cv_lib_pthread_sem_init=yes
else
echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
ac_cv_lib_pthread_sem_init=no
fi
rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
conftest$ac_exeext conftest.$ac_ext
LIBS=$ac_check_lib_save_LIBS
fi
{ echo "$as_me:$LINENO: result: $ac_cv_lib_pthread_sem_init" >&5
echo "${ECHO_T}$ac_cv_lib_pthread_sem_init" >&6; }
if test $ac_cv_lib_pthread_sem_init = yes; then
cat >>confdefs.h <<\_ACEOF
#define HAVE_SEM_INIT 1
_ACEOF
SEM_INIT_LIB=-lpthread
else
{ echo "$as_me:$LINENO: checking for sem_init in -lrt" >&5
echo $ECHO_N "checking for sem_init in -lrt... $ECHO_C" >&6; }
if test "${ac_cv_lib_rt_sem_init+set}" = set; then
echo $ECHO_N "(cached) $ECHO_C" >&6
else
ac_check_lib_save_LIBS=$LIBS
LIBS="-lrt $LIBS"
cat >conftest.$ac_ext <<_ACEOF
/* confdefs.h. */
_ACEOF
cat confdefs.h >>conftest.$ac_ext
cat >>conftest.$ac_ext <<_ACEOF
/* end confdefs.h. */
/* Override any GCC internal prototype to avoid an error.
Use char because int might match the return type of a GCC
builtin and then its argument prototype would still apply. */
#ifdef __cplusplus
extern "C"
#endif
char sem_init ();
int
main ()
{
return sem_init ();
;
return 0;
}
_ACEOF
rm -f conftest.$ac_objext conftest$ac_exeext
if { (ac_try="$ac_link"
case "(($ac_try" in
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
*) ac_try_echo=$ac_try;;
esac
eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
(eval "$ac_link") 2>conftest.er1
ac_status=$?
grep -v '^ *+' conftest.er1 >conftest.err
rm -f conftest.er1
cat conftest.err >&5
echo "$as_me:$LINENO: \$? = $ac_status" >&5
(exit $ac_status); } && {
test -z "$ac_c_werror_flag" ||
test ! -s conftest.err
} && test -s conftest$ac_exeext &&
$as_test_x conftest$ac_exeext; then
ac_cv_lib_rt_sem_init=yes
else
echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
ac_cv_lib_rt_sem_init=no
fi
rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
conftest$ac_exeext conftest.$ac_ext
LIBS=$ac_check_lib_save_LIBS
fi
{ echo "$as_me:$LINENO: result: $ac_cv_lib_rt_sem_init" >&5
echo "${ECHO_T}$ac_cv_lib_rt_sem_init" >&6; }
if test $ac_cv_lib_rt_sem_init = yes; then
cat >>confdefs.h <<\_ACEOF
#define HAVE_SEM_INIT 1
_ACEOF
SEM_INIT_LIB=-lrt
else
{ echo "$as_me:$LINENO: checking for sem_init in -lposix4" >&5
echo $ECHO_N "checking for sem_init in -lposix4... $ECHO_C" >&6; }
if test "${ac_cv_lib_posix4_sem_init+set}" = set; then
echo $ECHO_N "(cached) $ECHO_C" >&6
else
ac_check_lib_save_LIBS=$LIBS
LIBS="-lposix4 $LIBS"
cat >conftest.$ac_ext <<_ACEOF
/* confdefs.h. */
_ACEOF
cat confdefs.h >>conftest.$ac_ext
cat >>conftest.$ac_ext <<_ACEOF
/* end confdefs.h. */
/* Override any GCC internal prototype to avoid an error.
Use char because int might match the return type of a GCC
builtin and then its argument prototype would still apply. */
#ifdef __cplusplus
extern "C"
#endif
char sem_init ();
int
main ()
{
return sem_init ();
;
return 0;
}
_ACEOF
rm -f conftest.$ac_objext conftest$ac_exeext
if { (ac_try="$ac_link"
case "(($ac_try" in
*\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
*) ac_try_echo=$ac_try;;
esac
eval "echo \"\$as_me:$LINENO: $ac_try_echo\"") >&5
(eval "$ac_link") 2>conftest.er1
ac_status=$?
grep -v '^ *+' conftest.er1 >conftest.err
rm -f conftest.er1
cat conftest.err >&5
echo "$as_me:$LINENO: \$? = $ac_status" >&5
(exit $ac_status); } && {
test -z "$ac_c_werror_flag" ||
test ! -s conftest.err
} && test -s conftest$ac_exeext &&
$as_test_x conftest$ac_exeext; then
ac_cv_lib_posix4_sem_init=yes
else
echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5
ac_cv_lib_posix4_sem_init=no
fi
rm -f core conftest.err conftest.$ac_objext conftest_ipa8_conftest.oo \
conftest$ac_exeext conftest.$ac_ext
LIBS=$ac_check_lib_save_LIBS
fi
{ echo "$as_me:$LINENO: result: $ac_cv_lib_posix4_sem_init" >&5
echo "${ECHO_T}$ac_cv_lib_posix4_sem_init" >&6; }
if test $ac_cv_lib_posix4_sem_init = yes; then
cat >>confdefs.h <<\_ACEOF
#define HAVE_SEM_INIT 1
_ACEOF
SEM_INIT_LIB=-lposix4
fi
fi
fi
fi
{ echo "$as_me:$LINENO: checking for unified diff option" >&5
echo $ECHO_N "checking for unified diff option... $ECHO_C" >&6; }
if diff -u $0 $0 > /dev/null 2>&1 ; then
@ -16050,6 +16348,7 @@ SIZEOF_LONG!$SIZEOF_LONG$ac_delim
SIZEOF_LONG_LONG!$SIZEOF_LONG_LONG$ac_delim
SOCKET_LIB!$SOCKET_LIB$ac_delim
DLOPEN_LIB!$DLOPEN_LIB$ac_delim
SEM_INIT_LIB!$SEM_INIT_LIB$ac_delim
UNI_DIFF_OPTS!$UNI_DIFF_OPTS$ac_delim
LINUX_CMT!$LINUX_CMT$ac_delim
CYGWIN_CMT!$CYGWIN_CMT$ac_delim
@ -16070,7 +16369,7 @@ LIBOBJS!$LIBOBJS$ac_delim
LTLIBOBJS!$LTLIBOBJS$ac_delim
_ACEOF
if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 83; then
if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 84; then
break
elif $ac_last_try; then
{ { echo "$as_me:$LINENO: error: could not make $CONFIG_STATUS" >&5

View File

@ -584,7 +584,7 @@ if test $cross_compiling = no; then
else
AC_CHECK_PROGS(BUILD_CC, gcc cc)
fi
AC_CHECK_HEADERS(dirent.h errno.h getopt.h malloc.h mntent.h paths.h setjmp.h signal.h stdarg.h stdint.h stdlib.h termios.h termio.h unistd.h utime.h linux/fd.h linux/major.h net/if_dl.h netinet/in.h sys/disklabel.h sys/file.h sys/ioctl.h sys/mkdev.h sys/mman.h sys/prctl.h sys/queue.h sys/resource.h sys/select.h sys/socket.h sys/sockio.h sys/stat.h sys/syscall.h sys/sysmacros.h sys/time.h sys/types.h sys/un.h sys/wait.h)
AC_CHECK_HEADERS(dirent.h errno.h getopt.h malloc.h mntent.h paths.h semaphore.h setjmp.h signal.h stdarg.h stdint.h stdlib.h termios.h termio.h unistd.h utime.h linux/fd.h linux/major.h net/if_dl.h netinet/in.h sys/disklabel.h sys/file.h sys/ioctl.h sys/mkdev.h sys/mman.h sys/prctl.h sys/queue.h sys/resource.h sys/select.h sys/socket.h sys/sockio.h sys/stat.h sys/syscall.h sys/sysmacros.h sys/time.h sys/types.h sys/un.h sys/wait.h)
AC_CHECK_HEADERS(sys/disk.h sys/mount.h,,,
[[
#if HAVE_SYS_QUEUE_H
@ -729,6 +729,21 @@ if test $ac_cv_have_optreset = yes; then
AC_DEFINE(HAVE_OPTRESET)
fi
dnl
dnl Test for sem_init, and which library it might require:
dnl
SEM_INIT_LIB=''
AC_CHECK_FUNC(sem_init, ,
AC_CHECK_LIB(pthread, sem_init,
AC_DEFINE(HAVE_SEM_INIT)
SEM_INIT_LIB=-lpthread,
AC_CHECK_LIB(rt, sem_init,
AC_DEFINE(HAVE_SEM_INIT)
SEM_INIT_LIB=-lrt,
AC_CHECK_LIB(posix4, sem_init,
AC_DEFINE(HAVE_SEM_INIT)
SEM_INIT_LIB=-lposix4))))dnl
AC_SUBST(SEM_INIT_LIB)
dnl
dnl Check for unified diff
dnl
AC_MSG_CHECKING(for unified diff option)

View File

@ -9,6 +9,8 @@ libcom_err.so.2 libcomerr2 #MINVER#
com_right@Base 1.34
error_message@Base 1.01
error_table_name@Base 1.01
et_list_lock@Base 1.41.2
et_list_unlock@Base 1.41.2
free_error_table@Base 1.34
init_error_table@Base 1.01
initialize_error_table_r@Base 1.34

View File

@ -30,7 +30,7 @@ ELF_SO_VERSION = 2
ELF_IMAGE = libcom_err
ELF_MYDIR = et
ELF_INSTALL_DIR = $(root_libdir)
ELF_OTHER_LIBS =
ELF_OTHER_LIBS = @SEM_INIT_LIB@
BSDLIB_VERSION = 1.1
BSDLIB_IMAGE = libcom_err

View File

@ -9,3 +9,4 @@ Version: @E2FSPROGS_VERSION@
Requires:
Cflags: -I${includedir}/et
Libs: -L${libdir} -lcom_err
Libs.private: @SEM_INIT_LIB@

View File

@ -28,6 +28,9 @@
#if (!defined(HAVE_PRCTL) && defined(linux))
#include <sys/syscall.h>
#endif
#ifdef HAVE_SEMAPHORE_H
#include <semaphore.h>
#endif
#if HAVE_UNISTD_H
#include <unistd.h>
#endif
@ -49,6 +52,49 @@ THREAD_LOCAL char buffer[25];
struct et_list * _et_list = (struct et_list *) NULL;
struct et_list * _et_dynamic_list = (struct et_list *) NULL;
#ifdef __GNUC__
#define COMERR_ATTR(x) __attribute__(x)
#else
#define COMERR_ATTR(x)
#endif
#ifdef HAVE_SEM_INIT
static sem_t _et_lock;
static _et_lock_initialized;
static COMERR_ATTR((constructor)) setup_et_lock()
{
sem_init(&_et_lock, 0, 1);
_et_lock_initialized = 1;
}
static COMERR_ATTR((destructor)) fini_et_lock()
{
sem_destroy(&_et_lock);
_et_lock_initialized = 0;
}
#endif
int et_list_lock()
{
#ifdef HAVE_SEM_INIT
if (!_et_lock_initialized)
setup_et_lock();
return sem_wait(&_et_lock);
#else
return 0;
#endif
}
int et_list_unlock()
{
#ifdef HAVE_SEM_INIT
if (_et_lock_initialized)
return sem_post(&_et_lock);
#endif
return 0;
}
const char * error_message (errcode_t code)
{
@ -74,22 +120,32 @@ const char * error_message (errcode_t code)
goto oops;
#endif
}
et_list_lock();
for (et = _et_list; et; et = et->next) {
if ((et->table->base & 0xffffffL) == (table_num & 0xffffffL)) {
/* This is the right table */
if (et->table->n_msgs <= offset)
goto oops;
return(et->table->msgs[offset]);
if (et->table->n_msgs <= offset) {
break;
} else {
const char *msg = et->table->msgs[offset];
et_list_unlock();
return msg;
}
}
}
for (et = _et_dynamic_list; et; et = et->next) {
if ((et->table->base & 0xffffffL) == (table_num & 0xffffffL)) {
/* This is the right table */
if (et->table->n_msgs <= offset)
goto oops;
return(et->table->msgs[offset]);
if (et->table->n_msgs <= offset) {
break;
} else {
const char *msg = et->table->msgs[offset];
et_list_unlock();
return msg;
}
}
}
et_list_unlock();
oops:
strcpy (buffer, "Unknown code ");
if (table_num) {
@ -176,6 +232,11 @@ errcode_t add_error_table(const struct error_table * et)
if (!(el = (struct et_list *) malloc(sizeof(struct et_list))))
return ENOMEM;
if (et_list_lock() != 0) {
free(el);
return errno;
}
el->table = et;
el->next = _et_dynamic_list;
_et_dynamic_list = el;
@ -186,6 +247,7 @@ errcode_t add_error_table(const struct error_table * et)
error_table_name(et->base),
(const void *) et);
et_list_unlock();
return 0;
}
@ -194,9 +256,13 @@ errcode_t add_error_table(const struct error_table * et)
*/
errcode_t remove_error_table(const struct error_table * et)
{
struct et_list *el = _et_dynamic_list;
struct et_list *el;
struct et_list *el2 = 0;
if (et_list_lock() != 0)
return ENOENT;
el = _et_dynamic_list;
init_debug();
while (el) {
if (el->table->base == et->base) {
@ -210,6 +276,7 @@ errcode_t remove_error_table(const struct error_table * et)
"remove_error_table: %s (0x%p)\n",
error_table_name(et->base),
(const void *) et);
et_list_unlock();
return 0;
}
el2 = el;
@ -219,6 +286,7 @@ errcode_t remove_error_table(const struct error_table * et)
fprintf(debug_f, "remove_error_table FAILED: %s (0x%p)\n",
error_table_name(et->base),
(const void *) et);
et_list_unlock();
return ENOENT;
}