[PATCH] libwbclient clear pointer on bad read

[Volker Lendecke]

On Tue, Feb 24, 2015 at 10:59:15PM +0000, Matthew Newton wrote:
> On Tue, Feb 24, 2015 at 12:17:12PM +0100, Volker Lendecke wrote:
> > On Mon, Feb 23, 2015 at 01:18:39AM +0000, Matthew Newton wrote:
> > > Well, after life got in the way for a bit, I've managed to spend a
> > > few evenings and finally worm on this. The patch is (obviously)
> > > quite a lot bigger than before, but hopefully still fairly clear.
> > 
> > Yes, this looks very, very good!
> > 
> > A few comments will follow.
> 
> I've updated and attached the patches per your notes - definitely
> the better way to do things, thanks!

Ok, this is almost there :-)

I've attached a patch with the changes I'd propose:

In wbclient.c you need a struct winbindd_context forward
declaration. Otherwise gcc gives nasty warnings, and you end
up with a different type. wbcContextDestructor() does not
need the cast anymore.

The third one is a functional problem: If you declare 

static struct wbcContext wbcGlobalCtx;

in wbclient_internal.h you end up with multiple variables,
one in each .o file. This is almost certainly not what you
want.

And the last comment: Can you copy&paste the doxygen
comments for the wbcCtx functions?

Thanks!

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
 nsswitch/libwbclient/wbc_pwd.c           |   14 +++++++-------
 nsswitch/libwbclient/wbclient.c          |    9 ++++++++-
 nsswitch/libwbclient/wbclient.h          |    6 ++++++
 nsswitch/libwbclient/wbclient_internal.h |    2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/nsswitch/libwbclient/wbc_pwd.c b/nsswitch/libwbclient/wbc_pwd.c
index d62b1b5..805ab63 100644
--- a/nsswitch/libwbclient/wbc_pwd.c
+++ b/nsswitch/libwbclient/wbc_pwd.c
@@ -371,7 +371,7 @@ wbcErr wbcCtxSetpwent(struct wbcContext *ctx)
 	wbcErr wbc_status = WBC_ERR_UNKNOWN_FAILURE;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	if (ctx->pw_cache_size > 0) {
@@ -400,7 +400,7 @@ wbcErr wbcCtxEndpwent(struct wbcContext *ctx)
 	wbcErr wbc_status = WBC_ERR_UNKNOWN_FAILURE;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	if (ctx->pw_cache_size > 0) {
@@ -429,7 +429,7 @@ wbcErr wbcCtxGetpwent(struct wbcContext *ctx, struct passwd **pwd)
 	struct winbindd_pw *wb_pw;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	/* If there's a cached result, return that. */
@@ -483,7 +483,7 @@ wbcErr wbcCtxSetgrent(struct wbcContext *ctx)
 	wbcErr wbc_status = WBC_ERR_UNKNOWN_FAILURE;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	if (ctx->gr_cache_size > 0) {
@@ -512,7 +512,7 @@ wbcErr wbcCtxEndgrent(struct wbcContext *ctx)
 	wbcErr wbc_status = WBC_ERR_UNKNOWN_FAILURE;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	if (ctx->gr_cache_size > 0) {
@@ -542,7 +542,7 @@ wbcErr wbcCtxGetgrent(struct wbcContext *ctx, struct group **grp)
 	uint32_t mem_ofs;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	/* If there's a cached result, return that. */
@@ -597,7 +597,7 @@ wbcErr wbcCtxGetgrlist(struct wbcContext *ctx, struct group **grp)
 	struct winbindd_gr *wb_gr;
 
 	if (!ctx) {
-		ctx = &wbcGlobalCtx;
+		ctx = wbcGetGlobalCtx();
 	}
 
 	/* If there's a cached result, return that. */
diff --git a/nsswitch/libwbclient/wbclient.c b/nsswitch/libwbclient/wbclient.c
index e7500ef..cba3e3d 100644
--- a/nsswitch/libwbclient/wbclient.c
+++ b/nsswitch/libwbclient/wbclient.c
@@ -28,6 +28,8 @@
 
 /* From wb_common.c */
 
+struct winbindd_context;
+
 NSS_STATUS winbindd_request_response(struct winbindd_context *wbctx,
 				     int req_type,
 				     struct winbindd_request *request,
@@ -49,6 +51,11 @@ static struct wbcContext wbcGlobalCtx = {
 	.gr_cache_idx = 0
 };
 
+struct wbcContext *wbcGetGlobalCtx(void)
+{
+	return &wbcGlobalCtx;
+}
+
 /*
  result == NSS_STATUS_UNAVAIL: winbind not around
  result == NSS_STATUS_NOTFOUND: winbind around, but domain missing
@@ -293,7 +300,7 @@ static void wbcContextDestructor(void *ptr)
 {
 	struct wbcContext *ctx = (struct wbcContext *)ptr;
 
-	winbindd_ctx_free((struct winbindd_context *) ctx->winbindd_ctx);
+	winbindd_ctx_free(ctx->winbindd_ctx);
 }
 
 struct wbcContext *wbcCtxCreate(void)
diff --git a/nsswitch/libwbclient/wbclient.h b/nsswitch/libwbclient/wbclient.h
index 4f28074..dc58abb 100644
--- a/nsswitch/libwbclient/wbclient.h
+++ b/nsswitch/libwbclient/wbclient.h
@@ -1353,6 +1353,7 @@ wbcErr wbcAuthenticateUserEx(const struct wbcAuthUserParams *params,
 /**
  * @brief Logon a User
  *
+ * @param *ctx             wbclient Context
  * @param[in]  params      Pointer to a wbcLogonUserParams structure
  * @param[out] info        Pointer to a pointer to a wbcLogonUserInfo structure
  * @param[out] error       Pointer to a pointer to a wbcAuthErrorInfo structure
@@ -1360,6 +1361,11 @@ wbcErr wbcAuthenticateUserEx(const struct wbcAuthUserParams *params,
  *
  * @return #wbcErr
  **/
+wbcErr wbcCtxLogonUser(struct wbcContext *ctx,
+		       const struct wbcLogonUserParams *params,
+		       struct wbcLogonUserInfo **info,
+		       struct wbcAuthErrorInfo **error,
+		       struct wbcUserPasswordPolicyInfo **policy);
 wbcErr wbcLogonUser(const struct wbcLogonUserParams *params,
 		    struct wbcLogonUserInfo **info,
 		    struct wbcAuthErrorInfo **error,
diff --git a/nsswitch/libwbclient/wbclient_internal.h b/nsswitch/libwbclient/wbclient_internal.h
index 3aabde1..55477ef 100644
--- a/nsswitch/libwbclient/wbclient_internal.h
+++ b/nsswitch/libwbclient/wbclient_internal.h
@@ -46,6 +46,6 @@ struct wbcContext {
 	uint32_t gr_cache_idx;  /* Position of the grent context */
 };
 
-static struct wbcContext wbcGlobalCtx;
+struct wbcContext *wbcGetGlobalCtx(void);
 
 #endif      /* _WBCLIENT_INTERNAL_H */