[PATCH 1/6] Make winbind client library thread-safe by adding context
Matthew Newton
mcn4 at leicester.ac.uk
Sun Feb 22 18:19:37 MST 2015
Rather than keep state in global variables, store the current
context such as the winbind file descriptor in a struct that is
passed in. This makes the winbind client library thread-safe.
Signed-off-by: Matthew Newton <matthew-git at newtoncomputing.co.uk>
---
nsswitch/wb_common.c | 180 ++++++++++++++++++++++----------
nsswitch/winbind_client.h | 21 ++--
nsswitch/winbind_struct_protocol.h | 9 ++
source4/torture/winbind/struct_based.c | 2 +-
4 files changed, 150 insertions(+), 62 deletions(-)
diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 44bfaf4..e2a209b 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -6,6 +6,7 @@
Copyright (C) Tim Potter 2000
Copyright (C) Andrew Tridgell 2000
Copyright (C) Andrew Bartlett 2002
+ Copyright (C) Matthew Newton 2015
This library is free software; you can redistribute it and/or
@@ -26,10 +27,13 @@
#include "system/select.h"
#include "winbind_client.h"
-/* Global variables. These are effectively the client state information */
+/* Global context */
-int winbindd_fd = -1; /* fd for winbindd socket */
-static int is_privileged = 0;
+static struct winbindd_context wb_global_ctx = {
+ .winbindd_fd = -1,
+ .is_privileged = 0,
+ .our_pid = 0
+};
/* Free a response structure */
@@ -64,15 +68,26 @@ static void init_response(struct winbindd_response *response)
/* Close established socket */
+static void winbind_close_sock(struct winbindd_context *ctx)
+{
+ if (!ctx) {
+ return;
+ }
+
+ if (ctx->winbindd_fd != -1) {
+ close(ctx->winbindd_fd);
+ ctx->winbindd_fd = -1;
+ }
+}
+
+/* Destructor for global context to ensure fd is closed */
+
#if HAVE_FUNCTION_ATTRIBUTE_DESTRUCTOR
__attribute__((destructor))
#endif
-static void winbind_close_sock(void)
+static void winbind_destructor(void)
{
- if (winbindd_fd != -1) {
- close(winbindd_fd);
- winbindd_fd = -1;
- }
+ winbind_close_sock(&wb_global_ctx);
}
#define CONNECT_TIMEOUT 30
@@ -333,43 +348,52 @@ static const char *winbindd_socket_dir(void)
/* Connect to winbindd socket */
-static int winbind_open_pipe_sock(int recursing, int need_priv)
+static int winbind_open_pipe_sock(struct winbindd_context *ctx,
+ int recursing, int need_priv)
{
#ifdef HAVE_UNIXSOCKET
- static pid_t our_pid;
struct winbindd_request request;
struct winbindd_response response;
+
ZERO_STRUCT(request);
ZERO_STRUCT(response);
- if (our_pid != getpid()) {
- winbind_close_sock();
- our_pid = getpid();
+ if (!ctx) {
+ return -1;
+ }
+
+ if (ctx->our_pid != getpid()) {
+ winbind_close_sock(ctx);
+ ctx->our_pid = getpid();
}
- if ((need_priv != 0) && (is_privileged == 0)) {
- winbind_close_sock();
+ if ((need_priv != 0) && (ctx->is_privileged == 0)) {
+ winbind_close_sock(ctx);
}
- if (winbindd_fd != -1) {
- return winbindd_fd;
+ if (ctx->winbindd_fd != -1) {
+ return ctx->winbindd_fd;
}
if (recursing) {
return -1;
}
- if ((winbindd_fd = winbind_named_pipe_sock(winbindd_socket_dir())) == -1) {
+ ctx->winbindd_fd = winbind_named_pipe_sock(winbindd_socket_dir());
+
+ if (ctx->winbindd_fd == -1) {
return -1;
}
- is_privileged = 0;
+ ctx->is_privileged = 0;
/* version-check the socket */
request.wb_flags = WBFLAG_RECURSE;
- if ((winbindd_request_response(WINBINDD_INTERFACE_VERSION, &request, &response) != NSS_STATUS_SUCCESS) || (response.data.interface_version != WINBIND_INTERFACE_VERSION)) {
- winbind_close_sock();
+ if ((winbindd_request_response(ctx, WINBINDD_INTERFACE_VERSION, &request,
+ &response) != NSS_STATUS_SUCCESS) ||
+ (response.data.interface_version != WINBIND_INTERFACE_VERSION)) {
+ winbind_close_sock(ctx);
return -1;
}
@@ -383,22 +407,24 @@ static int winbind_open_pipe_sock(int recursing, int need_priv)
* thus response.extra_data.data will not be NULL even though
* winbindd response did not write over it due to a failure */
ZERO_STRUCT(response);
- if (winbindd_request_response(WINBINDD_PRIV_PIPE_DIR, &request, &response) == NSS_STATUS_SUCCESS) {
+ if (winbindd_request_response(ctx, WINBINDD_PRIV_PIPE_DIR, &request,
+ &response) == NSS_STATUS_SUCCESS) {
int fd;
- if ((fd = winbind_named_pipe_sock((char *)response.extra_data.data)) != -1) {
- close(winbindd_fd);
- winbindd_fd = fd;
- is_privileged = 1;
+ fd = winbind_named_pipe_sock((char *)response.extra_data.data);
+ if (fd != -1) {
+ close(ctx->winbindd_fd);
+ ctx->winbindd_fd = fd;
+ ctx->is_privileged = 1;
}
}
- if ((need_priv != 0) && (is_privileged == 0)) {
+ if ((need_priv != 0) && (ctx->is_privileged == 0)) {
return -1;
}
SAFE_FREE(response.extra_data.data);
- return winbindd_fd;
+ return ctx->winbindd_fd;
#else
return -1;
#endif /* HAVE_UNIXSOCKET */
@@ -406,8 +432,8 @@ static int winbind_open_pipe_sock(int recursing, int need_priv)
/* Write data to winbindd socket */
-static int winbind_write_sock(void *buffer, int count, int recursing,
- int need_priv)
+static int winbind_write_sock(struct winbindd_context *ctx, void *buffer,
+ int count, int recursing, int need_priv)
{
int fd, result, nwritten;
@@ -415,7 +441,7 @@ static int winbind_write_sock(void *buffer, int count, int recursing,
restart:
- fd = winbind_open_pipe_sock(recursing, need_priv);
+ fd = winbind_open_pipe_sock(ctx, recursing, need_priv);
if (fd == -1) {
errno = ENOENT;
return -1;
@@ -437,7 +463,7 @@ static int winbind_write_sock(void *buffer, int count, int recursing,
ret = poll(&pfd, 1, -1);
if (ret == -1) {
- winbind_close_sock();
+ winbind_close_sock(ctx);
return -1; /* poll error */
}
@@ -447,7 +473,7 @@ static int winbind_write_sock(void *buffer, int count, int recursing,
/* Pipe has closed on remote end */
- winbind_close_sock();
+ winbind_close_sock(ctx);
goto restart;
}
@@ -460,7 +486,7 @@ static int winbind_write_sock(void *buffer, int count, int recursing,
/* Write failed */
- winbind_close_sock();
+ winbind_close_sock(ctx);
return -1;
}
@@ -472,13 +498,14 @@ static int winbind_write_sock(void *buffer, int count, int recursing,
/* Read data from winbindd socket */
-static int winbind_read_sock(void *buffer, int count)
+static int winbind_read_sock(struct winbindd_context *ctx,
+ void *buffer, int count)
{
int fd;
int nread = 0;
int total_time = 0;
- fd = winbind_open_pipe_sock(false, false);
+ fd = winbind_open_pipe_sock(ctx, false, false);
if (fd == -1) {
return -1;
}
@@ -498,7 +525,7 @@ static int winbind_read_sock(void *buffer, int count)
ret = poll(&pfd, 1, 5000);
if (ret == -1) {
- winbind_close_sock();
+ winbind_close_sock(ctx);
return -1; /* poll error */
}
@@ -506,7 +533,7 @@ static int winbind_read_sock(void *buffer, int count)
/* Not ready for read yet... */
if (total_time >= 30) {
/* Timeout */
- winbind_close_sock();
+ winbind_close_sock(ctx);
return -1;
}
total_time += 5;
@@ -526,7 +553,7 @@ static int winbind_read_sock(void *buffer, int count)
can do here is just return -1 and fail since the
transaction has failed half way through. */
- winbind_close_sock();
+ winbind_close_sock(ctx);
return -1;
}
@@ -540,7 +567,8 @@ static int winbind_read_sock(void *buffer, int count)
/* Read reply */
-static int winbindd_read_reply(struct winbindd_response *response)
+static int winbindd_read_reply(struct winbindd_context *ctx,
+ struct winbindd_response *response)
{
int result1, result2 = 0;
@@ -550,7 +578,7 @@ static int winbindd_read_reply(struct winbindd_response *response)
/* Read fixed length response */
- result1 = winbind_read_sock(response,
+ result1 = winbind_read_sock(ctx, response,
sizeof(struct winbindd_response));
if (result1 == -1) {
return -1;
@@ -578,7 +606,7 @@ static int winbindd_read_reply(struct winbindd_response *response)
return -1;
}
- result2 = winbind_read_sock(response->extra_data.data,
+ result2 = winbind_read_sock(ctx, response->extra_data.data,
extra_data_len);
if (result2 == -1) {
winbindd_free_response(response);
@@ -595,7 +623,8 @@ static int winbindd_read_reply(struct winbindd_response *response)
* send simple types of requests
*/
-NSS_STATUS winbindd_send_request(int req_type, int need_priv,
+NSS_STATUS winbindd_send_request(struct winbindd_context *ctx,
+ int req_type, int need_priv,
struct winbindd_request *request)
{
struct winbindd_request lrequest;
@@ -615,7 +644,7 @@ NSS_STATUS winbindd_send_request(int req_type, int need_priv,
winbindd_init_request(request, req_type);
- if (winbind_write_sock(request, sizeof(*request),
+ if (winbind_write_sock(ctx, request, sizeof(*request),
request->wb_flags & WBFLAG_RECURSE,
need_priv) == -1)
{
@@ -626,7 +655,7 @@ NSS_STATUS winbindd_send_request(int req_type, int need_priv,
}
if ((request->extra_len != 0) &&
- (winbind_write_sock(request->extra_data.data,
+ (winbind_write_sock(ctx, request->extra_data.data,
request->extra_len,
request->wb_flags & WBFLAG_RECURSE,
need_priv) == -1))
@@ -644,7 +673,8 @@ NSS_STATUS winbindd_send_request(int req_type, int need_priv,
* Get results from winbindd request
*/
-NSS_STATUS winbindd_get_response(struct winbindd_response *response)
+NSS_STATUS winbindd_get_response(struct winbindd_context *ctx,
+ struct winbindd_response *response)
{
struct winbindd_response lresponse;
@@ -656,7 +686,7 @@ NSS_STATUS winbindd_get_response(struct winbindd_response *response)
init_response(response);
/* Wait for reply */
- if (winbindd_read_reply(response) == -1) {
+ if (winbindd_read_reply(ctx, response) == -1) {
/* Set ENOENT for consistency. Required by some apps */
errno = ENOENT;
@@ -678,38 +708,78 @@ NSS_STATUS winbindd_get_response(struct winbindd_response *response)
/* Handle simple types of requests */
-NSS_STATUS winbindd_request_response(int req_type,
- struct winbindd_request *request,
- struct winbindd_response *response)
+NSS_STATUS winbindd_request_response(struct winbindd_context *ctx,
+ int req_type,
+ struct winbindd_request *request,
+ struct winbindd_response *response)
{
NSS_STATUS status = NSS_STATUS_UNAVAIL;
int count = 0;
+ struct winbindd_context *wb_ctx = ctx;
+
+ if (ctx == NULL) {
+ wb_ctx = &wb_global_ctx;
+ }
while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) {
- status = winbindd_send_request(req_type, 0, request);
+ status = winbindd_send_request(wb_ctx, req_type, 0, request);
if (status != NSS_STATUS_SUCCESS)
return(status);
- status = winbindd_get_response(response);
+ status = winbindd_get_response(wb_ctx, response);
count += 1;
}
return status;
}
-NSS_STATUS winbindd_priv_request_response(int req_type,
+NSS_STATUS winbindd_priv_request_response(struct winbindd_context *ctx,
+ int req_type,
struct winbindd_request *request,
struct winbindd_response *response)
{
NSS_STATUS status = NSS_STATUS_UNAVAIL;
int count = 0;
+ struct winbindd_context *wb_ctx;
+
+ if (ctx == NULL) {
+ wb_ctx = &wb_global_ctx;
+ }
while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) {
- status = winbindd_send_request(req_type, 1, request);
+ status = winbindd_send_request(wb_ctx, req_type, 1, request);
if (status != NSS_STATUS_SUCCESS)
return(status);
- status = winbindd_get_response(response);
+ status = winbindd_get_response(wb_ctx, response);
count += 1;
}
return status;
}
+
+/* Create and free winbindd context */
+
+struct winbindd_context *winbindd_ctx_create(void)
+{
+ struct winbindd_context *ctx;
+
+ ctx = malloc(sizeof(struct winbindd_context));
+
+ if (!ctx) {
+ return NULL;
+ }
+
+ ctx->winbindd_fd = -1;
+ ctx->is_privileged = 0;
+ ctx->our_pid = 0;
+
+ return ctx;
+}
+
+void winbindd_ctx_free(struct winbindd_context *ctx)
+{
+ winbind_close_sock(ctx);
+
+ if (ctx) {
+ free(ctx);
+ }
+}
diff --git a/nsswitch/winbind_client.h b/nsswitch/winbind_client.h
index 905a189..16e7418 100644
--- a/nsswitch/winbind_client.h
+++ b/nsswitch/winbind_client.h
@@ -6,6 +6,7 @@
Copyright (C) Tim Potter 2000
Copyright (C) Andrew Tridgell 2000
Copyright (C) Andrew Bartlett 2002
+ Copyright (C) Matthew Newton 2015
This library is free software; you can redistribute it and/or
@@ -29,15 +30,23 @@
#include "winbind_struct_protocol.h"
void winbindd_free_response(struct winbindd_response *response);
-NSS_STATUS winbindd_send_request(int req_type, int need_priv,
+NSS_STATUS winbindd_send_request(struct winbindd_context *ctx,
+ int req_type, int need_priv,
struct winbindd_request *request);
-NSS_STATUS winbindd_get_response(struct winbindd_response *response);
-NSS_STATUS winbindd_request_response(int req_type,
- struct winbindd_request *request,
- struct winbindd_response *response);
-NSS_STATUS winbindd_priv_request_response(int req_type,
+NSS_STATUS winbindd_get_response(struct winbindd_context *ctx,
+ struct winbindd_response *response);
+NSS_STATUS winbindd_request_response(struct winbindd_context *ctx,
+ int req_type,
+ struct winbindd_request *request,
+ struct winbindd_response *response);
+NSS_STATUS winbindd_priv_request_response(struct winbindd_context *ctx,
+ int req_type,
struct winbindd_request *request,
struct winbindd_response *response);
+
+struct winbindd_context *winbindd_ctx_create(void);
+void winbindd_ctx_free(struct winbindd_context *ctx);
+
#define winbind_env_set() \
(strcmp(getenv(WINBINDD_DONT_ENV)?getenv(WINBINDD_DONT_ENV):"0","1") == 0)
diff --git a/nsswitch/winbind_struct_protocol.h b/nsswitch/winbind_struct_protocol.h
index 0dffa4b..f818d3f 100644
--- a/nsswitch/winbind_struct_protocol.h
+++ b/nsswitch/winbind_struct_protocol.h
@@ -5,6 +5,7 @@
Copyright (C) Tim Potter 2000
Copyright (C) Gerald Carter 2006
+ Copyright (C) Matthew Newton 2015
You are free to use this interface definition in any way you see
fit, including without restriction, using this header in your own
@@ -508,4 +509,12 @@ struct winbindd_response {
} extra_data;
};
+/* Winbind context */
+
+struct winbindd_context {
+ int winbindd_fd; /* winbind file descriptor */
+ bool is_privileged; /* using the privileged socket? */
+ pid_t our_pid; /* calling process pid */
+};
+
#endif
diff --git a/source4/torture/winbind/struct_based.c b/source4/torture/winbind/struct_based.c
index d47d068..883b38c 100644
--- a/source4/torture/winbind/struct_based.c
+++ b/source4/torture/winbind/struct_based.c
@@ -29,7 +29,7 @@
#define DO_STRUCT_REQ_REP_EXT(op,req,rep,expected,strict,warnaction,cmt) do { \
NSS_STATUS __got, __expected = (expected); \
- __got = winbindd_request_response(op, req, rep); \
+ __got = winbindd_request_response(NULL, op, req, rep); \
if (__got != __expected) { \
const char *__cmt = (cmt); \
if (strict) { \
--
1.7.10.4
--
Matthew Newton, Ph.D. <mcn4 at le.ac.uk>
Systems Specialist, Infrastructure Services,
I.T. Services, University of Leicester, Leicester LE1 7RH, United Kingdom
For IT help contact helpdesk extn. 2253, <ithelp at le.ac.uk>
More information about the samba-technical
mailing list