From cc202324064ac36a008b85e3b1946e59d83cb8b4 Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Mon, 5 Jan 2015 14:48:40 -0500 Subject: [PATCH] Fix errno error buffers to not clobber errors The strerror_r API has two flavors depending on system options. The bad flavor uses a static buffer for returning results, so if you save the pointer from strerror_r, the string you're referencing becomes useless if anybody else calls strerror_r again The good flavor does what you expect: it writes the error to your buffer. This commit uses strerror_r directly if it's a good version or copies the static buffer into our private buffer if it's a bad version. Thanks to gemorin for explaining the problem and drafting a fix. Fixes #239 --- hiredis.c | 2 +- hiredis.h | 24 ++++++++++++++++++++++++ net.c | 2 +- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/hiredis.c b/hiredis.c index 9a826ae..4b4c11a 100644 --- a/hiredis.c +++ b/hiredis.c @@ -579,7 +579,7 @@ void __redisSetError(redisContext *c, int type, const char *str) { } else { /* Only REDIS_ERR_IO may lack a description! */ assert(type == REDIS_ERR_IO); - strerror_r(errno,c->errstr,sizeof(c->errstr)); + __redis_strerror_r(errno, c->errstr, sizeof(c->errstr)); } } diff --git a/hiredis.h b/hiredis.h index f01b1ca..0a603fd 100644 --- a/hiredis.h +++ b/hiredis.h @@ -76,6 +76,30 @@ * SO_REUSEADDR is being used. */ #define REDIS_CONNECT_RETRIES 10 +/* strerror_r has two completely different prototypes and behaviors + * depending on system issues, so we need to operate on the error buffer + * differently depending on which strerror_r we're using. */ +#ifndef _GNU_SOURCE +/* "regular" POSIX strerror_r that does the right thing. */ +#define __redis_strerror_r(errno, buf, len) \ + do { \ + strerror_r((errno), (buf), (len)); \ + } while (0) +#else +/* "bad" GNU strerror_r we need to clean up after. */ +#define __redis_strerror_r(errno, buf, len) \ + do { \ + char *err_str = strerror_r((errno), (buf), (len)); \ + /* If return value _isn't_ the start of the buffer we passed in, \ + * then GNU strerror_r returned an internal static buffer and we \ + * need to copy the result into our private buffer. */ \ + if (err_str != (buf)) { \ + buf[(len)] = '\0'; \ + strncat((buf), err_str, ((len) - 1)); \ + } \ + } while (0) +#endif + #ifdef __cplusplus extern "C" { #endif diff --git a/net.c b/net.c index 0059066..be7a047 100644 --- a/net.c +++ b/net.c @@ -67,7 +67,7 @@ static void __redisSetErrorFromErrno(redisContext *c, int type, const char *pref if (prefix != NULL) len = snprintf(buf,sizeof(buf),"%s: ",prefix); - strerror_r(errno,buf+len,sizeof(buf)-len); + __redis_strerror_r(errno, (char *)(buf + len), sizeof(buf) - len); __redisSetError(c,type,buf); }