[PATCH] Simplify & speed up wbinfo -p

Stefan Metzmacher metze at samba.org
Tue Apr 24 09:05:30 UTC 2018


Hi Volker,

looks good, but I found an existing memory leak and fixed it before.

Please review and push the attached complete set.

Thanks!
metze

Am 24.04.2018 um 10:43 schrieb Volker Lendecke via samba-technical:
> Hi!
> 
> This gets wbinfo -p down to 2 roundtrips into winbind on 1 connection.
> Formerly this was 2 connections and at least 4 roundtrips.
> 
> Review appreciated!
> 
> Thanks, Volker
> 

-------------- next part --------------
From dc164716ea2ee36c7b3810ae6246da3f31ad8ebf Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 24 Apr 2018 10:59:05 +0200
Subject: [PATCH 1/4] nsswitch: fix memory leak in winbind_open_pipe_sock()
 when the privileged pipe is not accessable.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13400

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 nsswitch/wb_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index d6746b4..da81734 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -420,14 +420,14 @@ static int winbind_open_pipe_sock(struct winbindd_context *ctx,
 			ctx->winbindd_fd = fd;
 			ctx->is_privileged = 1;
 		}
+
+		SAFE_FREE(response.extra_data.data);
 	}
 
 	if ((need_priv != 0) && (ctx->is_privileged == 0)) {
 		return -1;
 	}
 
-	SAFE_FREE(response.extra_data.data);
-
 	return ctx->winbindd_fd;
 #else
 	return -1;
-- 
1.9.1


From 08a655562fd0e2f49ed1be44b034bbfbfba51a55 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 23 Apr 2018 14:04:48 +0200
Subject: [PATCH 2/4] ntlm_auth: PAM_AUTH_CRAP needs a privileged socket

This only works right now because wb_common always tries privileged

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 source3/utils/ntlm_auth.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c
index 2c8d991..d3146cc 100644
--- a/source3/utils/ntlm_auth.c
+++ b/source3/utils/ntlm_auth.c
@@ -593,7 +593,11 @@ NTSTATUS contact_winbind_auth_crap(const char *username,
                 request.data.auth_crap.nt_resp_len = nt_response->length;
 	}
 
-	result = winbindd_request_response(NULL, WINBINDD_PAM_AUTH_CRAP, &request, &response);
+	result = winbindd_priv_request_response(
+		NULL,
+		WINBINDD_PAM_AUTH_CRAP,
+		&request,
+		&response);
 	SAFE_FREE(request.extra_data.data);
 
 	/* Display response */
-- 
1.9.1


From 01f1de04fc7444fbaabcd8abfa2e7b6d3f80a66e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 23 Apr 2018 12:13:40 +0200
Subject: [PATCH 3/4] nsswitch: Only connect to the priv socket if required

This should speed up calls like "wbinfo -p"

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 nsswitch/wb_common.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index da81734..6768fde 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -401,6 +401,10 @@ static int winbind_open_pipe_sock(struct winbindd_context *ctx,
 		return -1;
 	}
 
+	if (need_priv == 0) {
+		return ctx->winbindd_fd;
+	}
+
 	/* try and get priv pipe */
 
 	request.wb_flags = WBFLAG_RECURSE;
@@ -424,7 +428,7 @@ static int winbind_open_pipe_sock(struct winbindd_context *ctx,
 		SAFE_FREE(response.extra_data.data);
 	}
 
-	if ((need_priv != 0) && (ctx->is_privileged == 0)) {
+	if (ctx->is_privileged == 0) {
 		return -1;
 	}
 
-- 
1.9.1


From fdbc4507cfd61e6e8d0d1fb960aba6cb650cd7b4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 18 Apr 2018 17:29:51 +0200
Subject: [PATCH 4/4] winbind: Speed up wbinfo -p

This was (possibly) used as an example in the early days of the async winbind
code we have today. It's not necessary to send this through a full tevent_req
round.

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 source3/winbindd/wb_ping.c        | 46 ---------------------------------------
 source3/winbindd/winbindd.c       |  3 +--
 source3/winbindd/winbindd_misc.c  |  6 +++++
 source3/winbindd/winbindd_proto.h |  6 -----
 source3/winbindd/wscript_build    |  1 -
 5 files changed, 7 insertions(+), 55 deletions(-)
 delete mode 100644 source3/winbindd/wb_ping.c

diff --git a/source3/winbindd/wb_ping.c b/source3/winbindd/wb_ping.c
deleted file mode 100644
index bfba3c1..0000000
--- a/source3/winbindd/wb_ping.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/*
-   Unix SMB/CIFS implementation.
-   async implementation of WINBINDD_PING
-   Copyright (C) Volker Lendecke 2009
-
-   This program is free software; you can redistribute it and/or modify
-   it under the terms of the GNU General Public License as published by
-   the Free Software Foundation; either version 3 of the License, or
-   (at your option) any later version.
-
-   This program is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-   GNU General Public License for more details.
-
-   You should have received a copy of the GNU General Public License
-   along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include "includes.h"
-#include "winbindd.h"
-
-struct wb_ping_state {
-	uint8_t dummy;
-};
-
-struct tevent_req *wb_ping_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
-				struct winbindd_cli_state *cli,
-				struct winbindd_request *request)
-{
-	struct tevent_req *req;
-	struct wb_ping_state *state;
-
-	req = tevent_req_create(mem_ctx, &state, struct wb_ping_state);
-	if (req == NULL) {
-		return NULL;
-	}
-	tevent_req_done(req);
-	tevent_req_post(req, ev);
-	return req;
-}
-
-NTSTATUS wb_ping_recv(struct tevent_req *req, struct winbindd_response *presp)
-{
-	return NT_STATUS_OK;
-}
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index b908d91e..76d644b 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -535,6 +535,7 @@ static struct winbindd_dispatch_table {
 	/* Miscellaneous */
 
 	{ WINBINDD_INFO, winbindd_info, "INFO" },
+	{ WINBINDD_PING, winbindd_ping, "PING" },
 	{ WINBINDD_INTERFACE_VERSION, winbindd_interface_version,
 	  "INTERFACE_VERSION" },
 	{ WINBINDD_DOMAIN_NAME, winbindd_domain_name, "DOMAIN_NAME" },
@@ -565,8 +566,6 @@ struct winbindd_async_dispatch_table {
 };
 
 static struct winbindd_async_dispatch_table async_nonpriv_table[] = {
-	{ WINBINDD_PING, "PING",
-	  wb_ping_send, wb_ping_recv },
 	{ WINBINDD_LOOKUPSID, "LOOKUPSID",
 	  winbindd_lookupsid_send, winbindd_lookupsid_recv },
 	{ WINBINDD_LOOKUPSIDS, "LOOKUPSIDS",
diff --git a/source3/winbindd/winbindd_misc.c b/source3/winbindd/winbindd_misc.c
index 964190e..c101269 100644
--- a/source3/winbindd/winbindd_misc.c
+++ b/source3/winbindd/winbindd_misc.c
@@ -502,6 +502,12 @@ void winbindd_dc_info(struct winbindd_cli_state *cli)
 	request_ok(cli);
 }
 
+void winbindd_ping(struct winbindd_cli_state *state)
+{
+	DEBUG(3, ("[%5lu]: ping\n", (unsigned long)state->pid));
+	request_ok(state);
+}
+
 /* List various tidbits of information */
 
 void winbindd_info(struct winbindd_cli_state *state)
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 6a63b15..bbc6841 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -529,12 +529,6 @@ bool parse_xidlist(TALLOC_CTX *mem_ctx, const char *xidstr,
 
 void winbindd_wins_byname(struct winbindd_cli_state *state);
 
-struct tevent_req *wb_ping_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
-				struct winbindd_cli_state *cli,
-				struct winbindd_request *request);
-NTSTATUS wb_ping_recv(struct tevent_req *req,
-		      struct winbindd_response *resp);
-
 enum winbindd_result winbindd_dual_ping(struct winbindd_domain *domain,
 					struct winbindd_cli_state *state);
 
diff --git a/source3/winbindd/wscript_build b/source3/winbindd/wscript_build
index 48250ea..0adbe9c 100644
--- a/source3/winbindd/wscript_build
+++ b/source3/winbindd/wscript_build
@@ -198,7 +198,6 @@ bld.SAMBA3_BINARY('winbindd',
                  winbindd_idmap.c
                  winbindd_locator.c
                  winbindd_ndr.c
-                 wb_ping.c
                  wb_lookupsid.c
                  wb_lookupsids.c
                  wb_lookupname.c
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180424/795ca58f/signature.sig>


More information about the samba-technical mailing list