From 27ae6c5b8b37a8086800cd1a4edbb01a7fddfad6 Mon Sep 17 00:00:00 2001

From: Simo Sorce <simo@redhat.com>

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 <gdeschner@redhat.com>

---

 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 <stdbool.h>

 #include <string.h>

 #include <stdlib.h>

+#include <stdio.h>

+#include <errno.h>

 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 <simo@redhat.com>

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 <gdeschner@redhat.com>

---

 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