From 27ae6c5b8b37a8086800cd1a4edbb01a7fddfad6 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 21 Nov 2013 11:59:40 -0500 Subject: [PATCH 1/2] Add Thread-safe implementation of strerror() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfortunately strerror() is not thread safe so we have to juggle with strerror_r() which is a can of worms as 2 incompatible implementations are available depending on what is defined at compile time. Try to do something sane. https://fedorahosted.org/gss-proxy/ticket/111 Reviewed-by: Günther Deschner --- proxy/src/gp_common.h | 3 +++ proxy/src/gp_util.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/proxy/src/gp_common.h b/proxy/src/gp_common.h index b5c525f..f2b8c3e 100644 --- a/proxy/src/gp_common.h +++ b/proxy/src/gp_common.h @@ -69,6 +69,9 @@ bool gp_same(const char *a, const char *b); bool gp_boolean_is_true(const char *s); char *gp_getenv(const char *name); +/* NOTE: read the note in gp_util.c before using gp_strerror() */ +char *gp_strerror(int errnum); + #include "rpcgen/gss_proxy.h" union gp_rpc_arg { diff --git a/proxy/src/gp_util.c b/proxy/src/gp_util.c index a6c870f..4fbac4e 100644 --- a/proxy/src/gp_util.c +++ b/proxy/src/gp_util.c @@ -27,6 +27,8 @@ #include #include #include +#include +#include bool gp_same(const char *a, const char *b) { @@ -66,3 +68,60 @@ char *gp_getenv(const char *name) return NULL; #endif } + +/* NOTE: because strerror_r() is such a mess with glibc, we need to do some + * magic checking to find out what function prototype is being used of the + * two incompatible ones, and pray it doesn't change in the future. + * On top of that to avoid impacting the current code too much we've got to use + * thread-local storage to hold a buffer. + * gp_strerror() is basically a thread-safe version of strerror() that can + * never fail. + */ +const char gp_internal_err[] = "Internal strerror_r() error."; +#define MAX_GP_STRERROR 1024 +char *gp_strerror(int errnum) +{ + static __thread char buf[MAX_GP_STRERROR]; + int saved_errno = errno; + +#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) + /* XSI version */ + int ret; + + ret = strerror_r(errnum, buf, MAX_GP_STRERROR); + if (ret == -1) ret = errno; + switch (ret) { + case 0: + break; + case EINVAL: + ret = snprintf(buf, MAX_GP_STRERROR, + "Unknown error code: %d", errnum); + if (ret > 0) break; + /* fallthrough */ + default: + ret = snprintf(buf, MAX_GP_STRERROR, + "Internal error describing error code: %d", errnum); + if (ret > 0) break; + memset(buf, 0, MAX_GP_STRERROR); + strncpy(buf, gp_internal_err, MAX_GP_STRERROR); + buf[MAX_GP_STRERROR -1] = '\0'; + } +#else + /* GNU-specific version */ + char *ret; + + ret = strerror_r(errnum, buf, MAX_GP_STRERROR); + if (ret == NULL) { + memset(buf, 0, MAX_GP_STRERROR); + strncpy(buf, gp_internal_err, MAX_GP_STRERROR); + buf[MAX_GP_STRERROR -1] = '\0'; + } else if (ret != buf) { + memset(buf, 0, MAX_GP_STRERROR); + strncpy(buf, ret, MAX_GP_STRERROR); + buf[MAX_GP_STRERROR -1] = '\0'; + } +#endif + + errno = saved_errno; + return buf; +} -- 1.8.3.1 From db8099da53167ca4ebf3b9f5ef0c702ddfe8b366 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 21 Nov 2013 12:14:36 -0500 Subject: [PATCH 2/2] Use gp_strerror() everywhere instead of strerror() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://fedorahosted.org/gss-proxy/ticket/111 Reviewed-by: Günther Deschner --- proxy/src/client/gpm_init_sec_context.c | 4 ++-- proxy/src/gp_config.c | 2 +- proxy/src/gp_config_dinglibs.c | 4 ++-- proxy/src/gp_init.c | 10 +++++----- proxy/src/gp_socket.c | 14 +++++++------- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/proxy/src/client/gpm_init_sec_context.c b/proxy/src/client/gpm_init_sec_context.c index f6dfe53..bd88055 100644 --- a/proxy/src/client/gpm_init_sec_context.c +++ b/proxy/src/client/gpm_init_sec_context.c @@ -90,7 +90,7 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status, /* execute proxy request */ ret = gpm_make_call(GSSX_INIT_SEC_CONTEXT, &uarg, &ures); if (ret) { - gpm_save_internal_status(ret, strerror(ret)); + gpm_save_internal_status(ret, gp_strerror(ret)); goto done; } @@ -114,7 +114,7 @@ OM_uint32 gpm_init_sec_context(OM_uint32 *minor_status, if (res->output_token) { ret = gp_conv_gssx_to_buffer_alloc(res->output_token, &outbuf); if (ret) { - gpm_save_internal_status(ret, strerror(ret)); + gpm_save_internal_status(ret, gp_strerror(ret)); goto done; } } diff --git a/proxy/src/gp_config.c b/proxy/src/gp_config.c index 63f264e..2fc4a6f 100644 --- a/proxy/src/gp_config.c +++ b/proxy/src/gp_config.c @@ -464,7 +464,7 @@ int load_config(struct gp_config *cfg) done: if (ret != 0) { - GPERROR("Error reading configuration %d: %s", ret, strerror(ret)); + GPERROR("Error reading configuration %d: %s", ret, gp_strerror(ret)); } gp_config_close(ctx); safefree(ctx); diff --git a/proxy/src/gp_config_dinglibs.c b/proxy/src/gp_config_dinglibs.c index 8716b91..515b951 100644 --- a/proxy/src/gp_config_dinglibs.c +++ b/proxy/src/gp_config_dinglibs.c @@ -238,7 +238,7 @@ int gp_dinglibs_init(const char *config_file, &file_ctx); if (ret) { GPDEBUG("Failed to open config file: %d (%s)\n", - ret, strerror(ret)); + ret, gp_strerror(ret)); ini_config_destroy(ini_config); return ret; } @@ -255,7 +255,7 @@ int gp_dinglibs_init(const char *config_file, char **errors = NULL; /* we had a parsing failure */ GPDEBUG("Failed to parse config file: %d (%s)\n", - ret, strerror(ret)); + ret, gp_strerror(ret)); if (ini_config_error_count(ini_config)) { ini_config_get_errors(ini_config, &errors); if (errors) { diff --git a/proxy/src/gp_init.c b/proxy/src/gp_init.c index 92c0a47..7e29c59 100644 --- a/proxy/src/gp_init.c +++ b/proxy/src/gp_init.c @@ -158,7 +158,7 @@ void init_proc_nfsd(struct gp_config *cfg) ret = errno; GPDEBUG("Failed to open %s: %d (%s)\n", LINUX_PROC_USE_GSS_PROXY_FILE, - ret, strerror(ret)); + ret, gp_strerror(ret)); return; } @@ -167,7 +167,7 @@ void init_proc_nfsd(struct gp_config *cfg) ret = errno; GPDEBUG("Failed to write to %s: %d (%s)\n", LINUX_PROC_USE_GSS_PROXY_FILE, - ret, strerror(ret)); + ret, gp_strerror(ret)); } ret = close(fd); @@ -175,7 +175,7 @@ void init_proc_nfsd(struct gp_config *cfg) ret = errno; GPDEBUG("Failed to close %s: %d (%s)\n", LINUX_PROC_USE_GSS_PROXY_FILE, - ret, strerror(ret)); + ret, gp_strerror(ret)); } } @@ -191,7 +191,7 @@ void write_pid(void) if (!f) { ret = errno; GPDEBUG("Failed to open %s: %d (%s)\n", - GP_PID_FILE, ret, strerror(ret)); + GP_PID_FILE, ret, gp_strerror(ret)); return; } @@ -204,6 +204,6 @@ void write_pid(void) if (ret != 0) { ret = errno; GPDEBUG("Failed to close %s: %d (%s)\n", - GP_PID_FILE, ret, strerror(ret)); + GP_PID_FILE, ret, gp_strerror(ret)); } } diff --git a/proxy/src/gp_socket.c b/proxy/src/gp_socket.c index b1851a2..3e8afc5 100644 --- a/proxy/src/gp_socket.c +++ b/proxy/src/gp_socket.c @@ -184,7 +184,7 @@ struct gp_sock_ctx *init_unix_socket(struct gssproxy_ctx *gpctx, fd = socket(AF_UNIX, SOCK_STREAM, 0); if (fd == -1) { ret = errno; - GPDEBUG("Failed to init socket! (%d: %s)\n", ret, strerror(ret)); + GPDEBUG("Failed to init socket! (%d: %s)\n", ret, gp_strerror(ret)); goto done; } @@ -192,14 +192,14 @@ struct gp_sock_ctx *init_unix_socket(struct gssproxy_ctx *gpctx, if (ret == -1) { ret = errno; GPDEBUG("Failed to bind socket %s! (%d: %s)\n", addr.sun_path, - ret, strerror(ret)); + ret, gp_strerror(ret)); goto done; } ret = listen(fd, 10); if (ret == -1) { ret = errno; - GPDEBUG("Failed to listen! (%d: %s)\n", ret, strerror(ret)); + GPDEBUG("Failed to listen! (%d: %s)\n", ret, gp_strerror(ret)); goto done; } @@ -218,7 +218,7 @@ struct gp_sock_ctx *init_unix_socket(struct gssproxy_ctx *gpctx, done: if (ret) { GPERROR("Failed to create Unix Socket! (%d:%s)", - ret, strerror(ret)); + ret, gp_strerror(ret)); if (fd != -1) { close(fd); fd = -1; @@ -245,7 +245,7 @@ static int get_peercred(int fd, struct gp_conn *conn) if (ret == -1) { ret = errno; GPDEBUG("Failed to get SO_PEERCRED options! (%d:%s)\n", - ret, strerror(ret)); + ret, gp_strerror(ret)); return ret; } if (len != sizeof(struct ucred)) { @@ -262,7 +262,7 @@ static int get_peercred(int fd, struct gp_conn *conn) } else { ret = errno; GPDEBUG("Failed to get peer's SELinux context (%d:%s)\n", - ret, strerror(ret)); + ret, gp_strerror(ret)); /* consider thisnot fatal, selinux may be disabled */ } @@ -579,7 +579,7 @@ void accept_sock_conn(verto_ctx *vctx, verto_ev *ev) done: if (ret) { GPERROR("Error connecting client: (%d:%s)", - ret, strerror(ret)); + ret, gp_strerror(ret)); gp_conn_free(conn); } } -- 1.8.3.1