[PATCH] Fix bug 13465

Jeremy Allison jra at samba.org
Tue Jul 10 20:20:21 UTC 2018


On Tue, Jul 10, 2018 at 10:03:58PM +0200, Ralph Böhme wrote:
> On Tue, Jul 10, 2018 at 12:29:48PM -0700, Christof Schmitt wrote:
> > On Tue, Jul 10, 2018 at 08:18:25AM +0200, Ralph Böhme wrote:
> > > On Mon, Jul 09, 2018 at 03:36:09PM -0700, Christof Schmitt wrote:
> > > >>fwiw, local reproducer:
> > > >>
> > > >>$ PYTHONPATH=bin/python python source4/scripting/bin/subunitrun
> > > >>samba.tests.docs
> > > >
> > > >This test calls testparm internally. The problem there is that
> > > >
> > > >bin/testparm -s paramtestsmb.conf -v | less
> > > >
> > > >shows an empty default for fstype with your patches applied.
> > > >Removing the call to client_messaging_context from testparm avoids
> > > >this problem and testparm again reports the NTFS default for
> > > >fstype. I will keep digging for the underlying cause.
> > > 
> > > I may have found it: <https://gitlab.com/samba-team/devel/samba/commit/daf6251fe84e81adc171982cf7488450451cdc23>
> > > 
> > > Currently running a private autobuild with it.
> > 
> > Good catch! I will also do some testing with your patches on my cluster.
> 
> please use the attached patchset which is quite different from the original
> approach.
> 
> Instead of initialiting messaging in all client tools individually, we must do
> it earlier in the popt callback, as in popt_common_credentials_callback() we
> already call lp_load_client(). This gets triggered by all client tools that use
> POPT_COMMON_CREDENTIALS which will still crash even when trying to initialitize
> messaging themself.
> 
> This passed CI in an older version with no functional differences:
> <https://gitlab.com/samba-team/devel/samba/pipelines/25486221>
> 
> The current patchset is short before passing:
> <https://gitlab.com/samba-team/devel/samba/pipelines/25493283>
> 
> Pleaese review & push if happy. Thanks!

Wow - great detective work Ralph !

Jeremy.

> -- 
> Ralph Boehme, Samba Team       https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
> GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
>                               59E4 AA1E 9B71 2639 9E46

> From 1ac2e58484cdc4d65cb457dde2e2b90cc46e2be4 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Mon, 9 Jul 2018 17:11:57 +0200
> Subject: [PATCH 01/12] s3: lib/server_contexts: make server_event_ctx and
>  server_msg_ctx static
> 
> server_event_ctx and server_msg_ctx static shouldn't be accessible from
> outside this compilation unit.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/lib/server_contexts.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/lib/server_contexts.c b/source3/lib/server_contexts.c
> index 50072e680b6..b21cf0a4c81 100644
> --- a/source3/lib/server_contexts.c
> +++ b/source3/lib/server_contexts.c
> @@ -21,7 +21,7 @@
>  #include "includes.h"
>  #include "messages.h"
>  
> -struct tevent_context *server_event_ctx = NULL;
> +static struct tevent_context *server_event_ctx = NULL;
>  
>  struct tevent_context *server_event_context(void)
>  {
> @@ -44,7 +44,7 @@ void server_event_context_free(void)
>  	TALLOC_FREE(server_event_ctx);
>  }
>  
> -struct messaging_context *server_msg_ctx = NULL;
> +static struct messaging_context *server_msg_ctx = NULL;
>  
>  struct messaging_context *server_messaging_context(void)
>  {
> -- 
> 2.13.6
> 
> 
> From eff2aab110fa7e824a5c8f501798cc96014e68a7 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 17:22:05 +0200
> Subject: [PATCH 02/12] s3: lib/server_contexts: rename server_event_ctx to ev
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/lib/server_contexts.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/source3/lib/server_contexts.c b/source3/lib/server_contexts.c
> index b21cf0a4c81..90001260c55 100644
> --- a/source3/lib/server_contexts.c
> +++ b/source3/lib/server_contexts.c
> @@ -21,27 +21,27 @@
>  #include "includes.h"
>  #include "messages.h"
>  
> -static struct tevent_context *server_event_ctx = NULL;
> +static struct tevent_context *ev = NULL;
>  
>  struct tevent_context *server_event_context(void)
>  {
> -	if (!server_event_ctx) {
> +	if (ev == NULL) {
>  		/*
>  		 * Note we MUST use the NULL context here, not the
>  		 * autofree context, to avoid side effects in forked
>  		 * children exiting.
>  		 */
> -		server_event_ctx = samba_tevent_context_init(NULL);
> +		ev = samba_tevent_context_init(NULL);
>  	}
> -	if (!server_event_ctx) {
> +	if (ev == NULL) {
>  		smb_panic("Could not init server's event context");
>  	}
> -	return server_event_ctx;
> +	return ev;
>  }
>  
>  void server_event_context_free(void)
>  {
> -	TALLOC_FREE(server_event_ctx);
> +	TALLOC_FREE(ev);
>  }
>  
>  static struct messaging_context *server_msg_ctx = NULL;
> -- 
> 2.13.6
> 
> 
> From 857403287fb703898b7560771ab030059a534902 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 17:25:27 +0200
> Subject: [PATCH 03/12] s3: lib/server_contexts: rename server_msg_ctx to
>  msg_ctx
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/lib/server_contexts.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/source3/lib/server_contexts.c b/source3/lib/server_contexts.c
> index 90001260c55..ce9b0ed1b88 100644
> --- a/source3/lib/server_contexts.c
> +++ b/source3/lib/server_contexts.c
> @@ -44,23 +44,22 @@ void server_event_context_free(void)
>  	TALLOC_FREE(ev);
>  }
>  
> -static struct messaging_context *server_msg_ctx = NULL;
> +static struct messaging_context *msg_ctx = NULL;
>  
>  struct messaging_context *server_messaging_context(void)
>  {
> -	if (server_msg_ctx == NULL) {
> +	if (msg_ctx == NULL) {
>  		/*
>  		 * Note we MUST use the NULL context here, not the
>  		 * autofree context, to avoid side effects in forked
>  		 * children exiting.
>  		 */
> -		server_msg_ctx = messaging_init(NULL,
> -					        server_event_context());
> +		msg_ctx = messaging_init(NULL, server_event_context());
>  	}
> -	return server_msg_ctx;
> +	return msg_ctx;
>  }
>  
>  void server_messaging_context_free(void)
>  {
> -	TALLOC_FREE(server_msg_ctx);
> +	TALLOC_FREE(msg_ctx);
>  }
> -- 
> 2.13.6
> 
> 
> From c6da6ac66858b4f4e12d8000679a37994f7ecdfe Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Mon, 9 Jul 2018 17:13:28 +0200
> Subject: [PATCH 04/12] s3: lib/server_contexts: add client event and messaging
>  context
> 
> This is similar to the existing server_contexts, but for clients like
> smbpasswd, testparm asf.
> 
> client_messaging_context() calls lp_load_initial_only() which is needed
> to initialize messaging when checking and creating the directories used
> for the messaging sockets and lockfiles.
> 
> Note that internally client and server event and messaging context must
> be the same, as low level in db_open() we call
> server_messaging_context().
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/include/proto.h       |  5 +++++
>  source3/lib/server_contexts.c | 48 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index fea4ba51be5..d5f8c7130fa 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -893,6 +893,11 @@ void server_event_context_free(void);
>  struct messaging_context *server_messaging_context(void);
>  void server_messaging_context_free(void);
>  
> +struct tevent_context *client_event_context(void);
> +void client_event_context_free(void);
> +struct messaging_context *client_messaging_context(const char *config_file);
> +void client_messaging_context_free(void);
> +
>  /* The following definitions come from lib/sessionid_tdb.c  */
>  struct sessionid;
>  NTSTATUS sessionid_traverse_read(int (*fn)(const char *key,
> diff --git a/source3/lib/server_contexts.c b/source3/lib/server_contexts.c
> index ce9b0ed1b88..1586a6535b6 100644
> --- a/source3/lib/server_contexts.c
> +++ b/source3/lib/server_contexts.c
> @@ -63,3 +63,51 @@ void server_messaging_context_free(void)
>  {
>  	TALLOC_FREE(msg_ctx);
>  }
> +
> +struct tevent_context *client_event_context(void)
> +{
> +	if (ev != NULL) {
> +		return ev;
> +	}
> +
> +	/*
> +	 * Note we MUST use the NULL context here, not the
> +	 * autofree context, to avoid side effects in forked
> +	 * children exiting.
> +	 */
> +	ev = samba_tevent_context_init(NULL);
> +	if (ev == NULL) {
> +		smb_panic("Could not init client's event context");
> +	}
> +	return ev;
> +}
> +
> +void client_event_context_free(void)
> +{
> +	TALLOC_FREE(ev);
> +}
> +
> +struct messaging_context *client_messaging_context(const char *config_file)
> +{
> +	if (msg_ctx != NULL) {
> +		return msg_ctx;
> +	}
> +
> +	if (!lp_load_initial_only(config_file)) {
> +		return NULL;
> +	}
> +
> +	/*
> +	 * Note we MUST use the NULL context here, not the
> +	 * autofree context, to avoid side effects in forked
> +	 * children exiting.
> +	 */
> +	msg_ctx = messaging_init(NULL, client_event_context());
> +
> +	return msg_ctx;
> +}
> +
> +void client_messaging_context_free(void)
> +{
> +	TALLOC_FREE(msg_ctx);
> +}
> -- 
> 2.13.6
> 
> 
> From e4dcd74d80424630e6e26f2d3aba1e91d386b23c Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 08:11:31 +0200
> Subject: [PATCH 05/12] s3: loadparm: reinit_globals in
>  lp_load_with_registry_shares()
> 
> This was set to false in 0e0d77519c27038b30fec92d542198e97be767d9 based
> on the assumption that callers would have no need to call
> lp_load_initial_only() with a later call to lp_load_something().
> 
> This is not quite correct, since for accessing registry config on a
> cluster with include=registry, we need messaging up and running which
> *itself* requires loadparm to be initialized to get the statedir,
> lockdir asf. directories.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/param/loadparm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
> index 5f646d63ce0..419134694cc 100644
> --- a/source3/param/loadparm.c
> +++ b/source3/param/loadparm.c
> @@ -4119,7 +4119,7 @@ bool lp_load_with_registry_shares(const char *pszFname)
>  			  false, /* global_only */
>  			  true,  /* save_defaults */
>  			  false, /* add_ipc */
> -			  false, /* reinit_globals */
> +			  true, /* reinit_globals */
>  			  true,  /* allow_include_registry */
>  			  true); /* load_all_shares*/
>  }
> -- 
> 2.13.6
> 
> 
> From 174159c802fd33665bc2fd5e750f159d7cc27ea9 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 10:38:10 +0200
> Subject: [PATCH 06/12] selftest: pass configfile to pdbedit
> 
> This is needed otherwise pdbedit fails to initialize messaging in
> autobuild.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  testprogs/blackbox/test_pdbtest.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/testprogs/blackbox/test_pdbtest.sh b/testprogs/blackbox/test_pdbtest.sh
> index 2ffded9af4e..02615094451 100755
> --- a/testprogs/blackbox/test_pdbtest.sh
> +++ b/testprogs/blackbox/test_pdbtest.sh
> @@ -44,12 +44,12 @@ send ${NEWUSERPASS}\n
>  send ${NEWUSERPASS}\n
>  EOF
>  
> -testit "create user with pdbedit" $texpect ./tmpsmbpasswdscript $VALGRIND $pdbedit -a $USER --account-desc="pdbedit-test-user" $@ || failed=`expr $failed + 1`
> +testit "create user with pdbedit" $texpect ./tmpsmbpasswdscript $VALGRIND $pdbedit -s $SMB_CONF -a $USER --account-desc="pdbedit-test-user" $@ || failed=`expr $failed + 1`
>  USERPASS=$NEWUSERPASS
>  
>  test_smbclient "Test login with user (ntlm)" 'ls' "$unc" -k no -U$USER%$NEWUSERPASS $@ || failed=`expr $failed + 1`
>  
> -testit "modify user"  $VALGRIND $pdbedit --modify $USER --drive="D:" $@ || failed=`expr $failed + 1`
> +testit "modify user"  $VALGRIND $pdbedit -s $SMB_CONF --modify $USER --drive="D:" $@ || failed=`expr $failed + 1`
>  
>  test_smbclient "Test login with user (ntlm)" 'ls' "$unc" -k no -U$USER%$NEWUSERPASS $@|| failed=`expr $failed + 1`
>  
> @@ -87,11 +87,11 @@ test_smbclient "Test login with no expiry (ntlm)" 'ls' "$unc" -k no -U$USER%$NEW
>  NEWUSERPASS=testPaSS at 03%
>  NEWUSERHASH=062519096c45739c1938800f80906731
>  
> -testit "Set user password with password hash" $VALGRIND $pdbedit -u $USER --set-nt-hash $NEWUSERHASH $@ || failed=`expr $failed + 1`
> +testit "Set user password with password hash" $VALGRIND $pdbedit -s $SMB_CONF -u $USER --set-nt-hash $NEWUSERHASH $@ || failed=`expr $failed + 1`
>  
>  test_smbclient "Test login with new password (from hash)" 'ls' "$unc" -k no -U$USER%$NEWUSERPASS || failed=`expr $failed + 1`
>  
> -testit "del user"  $VALGRIND $pdbedit -x $USER $@ || failed=`expr $failed + 1`
> +testit "del user"  $VALGRIND $pdbedit -s $SMB_CONF -x $USER $@ || failed=`expr $failed + 1`
>  
>  rm ./tmpsmbpasswdscript
>  
> -- 
> 2.13.6
> 
> 
> From 673236855b92a5094533f1d21898bb1de0fa7936 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 15:26:40 +0200
> Subject: [PATCH 07/12] s3: popt_common: use client_messaging_context()
> 
> This adds calls to client_messaging_context() to the popt hooks and
> ensure any client tool that uses POPT_COMMON_SAMBA or
> POPT_COMMON_CREDENTIALS gets an implicit messaging context, ensuring it
> doesn't crash in lp_load_*() which, with include=registry in smb.conf on
> a cluster, in most lp_load_*() cases ends up loading the registry
> config.
> 
> It crashes in the registry config initialisation because
> messaging_ctdb_connection() panics with a missing ctdb messaging
> connection.
> 
> It's necessary to inititalize messaging in the popt callbacks, because
> popt_common_credentials_callback() already calls lp_load_client() which
> triggers registry config initilisation, so doing it individually in main
> in the client tools is too late.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/lib/popt_common.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/source3/lib/popt_common.c b/source3/lib/popt_common.c
> index cc93a756c3b..1a1b0f3b364 100644
> --- a/source3/lib/popt_common.c
> +++ b/source3/lib/popt_common.c
> @@ -81,6 +81,7 @@ static void popt_common_callback(poptContext con,
>  	}
>  
>  	if (reason == POPT_CALLBACK_REASON_POST) {
> +		struct messaging_context *msg_ctx = NULL;
>  
>  		if (PrintSambaVersionString) {
>  			printf( "Version %s\n", samba_version_string());
> @@ -93,6 +94,14 @@ static void popt_common_callback(poptContext con,
>  			}
>  		}
>  
> +		msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
> +		if (msg_ctx == NULL) {
> +			fprintf(stderr, "Unable to initialize messaging context.");
> +			if (geteuid() == 0) {
> +				exit(1);
> +			}
> +		}
> +
>  		/* Further 'every Samba program must do this' hooks here. */
>  		return;
>  	}
> @@ -287,11 +296,20 @@ static void popt_common_credentials_callback(poptContext con,
>  
>  	if (reason == POPT_CALLBACK_REASON_POST) {
>  		bool ok;
> +		struct messaging_context *msg_ctx = NULL;
>  
>  		if (override_logfile) {
>  			setup_logging(lp_logfile(talloc_tos()), DEBUG_FILE );
>  		}
>  
> +		msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
> +		if (msg_ctx == NULL) {
> +			fprintf(stderr, "Unable to initialize messaging context.");
> +			if (geteuid() == 0) {
> +				exit(1);
> +			}
> +		}
> +
>  		ok = lp_load_client(get_dyn_CONFIGFILE());
>  		if (!ok) {
>  			const char *pname = poptGetInvocationName(con);
> -- 
> 2.13.6
> 
> 
> From 0f61891bf8edd7fbeb1e00e691702ff92520b6e5 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Mon, 9 Jul 2018 16:25:02 +0200
> Subject: [PATCH 08/12] s3: smbpasswd: use client_messaging_context
> 
> process_options() doesn't use getopt(), not popt with its
> autoinitialisation of a messaging context, so we have to call
> client_messaging_context() by hand.
> 
> The commit uses the correct new client_messaging_context() and moves the
> call the right place *after* popt processing which may set smb.conf from
> the cmdline parameter.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/utils/smbpasswd.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
> index 04f34aa9b69..a7db3c1ec84 100644
> --- a/source3/utils/smbpasswd.c
> +++ b/source3/utils/smbpasswd.c
> @@ -88,6 +88,7 @@ static int process_options(int argc, char **argv, int local_flags)
>  {
>  	int ch;
>  	const char *configfile = get_dyn_CONFIGFILE();
> +	struct messaging_context *msg_ctx = NULL;
>  
>  	local_flags |= LOCAL_SET_PASSWORD;
>  
> @@ -196,6 +197,18 @@ static int process_options(int argc, char **argv, int local_flags)
>  		usage();
>  	}
>  
> +	msg_ctx = client_messaging_context(configfile);
> +	if (msg_ctx == NULL) {
> +		if (geteuid() == 0) {
> +			fprintf(stderr,
> +				"Unable not able to initialize the "
> +				"messaging context!\n");
> +			exit(1);
> +		}
> +		DBG_NOTICE("Unable to initialize messaging context. "
> +			   "Must be root to do that.\n");
> +	}
> +
>  	if (!lp_load_global(configfile)) {
>  		fprintf(stderr, "Can't load %s - run testparm to debug it\n", 
>  			configfile);
> @@ -614,7 +627,6 @@ static int process_nonroot(int local_flags)
>  int main(int argc, char **argv)
>  {	
>  	TALLOC_CTX *frame = talloc_stackframe();
> -	struct messaging_context *msg_ctx = NULL;
>  	int local_flags = 0;
>  	int ret;
>  
> @@ -632,19 +644,6 @@ int main(int argc, char **argv)
>  
>  	setup_logging("smbpasswd", DEBUG_STDERR);
>  
> -	msg_ctx = server_messaging_context();
> -	if (msg_ctx == NULL) {
> -		if (geteuid() != 0) {
> -			DBG_NOTICE("Unable to initialize messaging context. "
> -				   "Must be root to do that.\n");
> -		} else {
> -			fprintf(stderr,
> -				"smbpasswd is not able to initialize the "
> -				"messaging context!\n");
> -			return 1;
> -		}
> -	}
> -
>  	/*
>  	 * Set the machine NETBIOS name if not already
>  	 * set from the config file. 
> -- 
> 2.13.6
> 
> 
> From 09f467ece9cc9c8ec46a9d81158181201c9390d1 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Mon, 9 Jul 2018 17:32:36 +0200
> Subject: [PATCH 09/12] s3: smbstatus: use client_messaging_context()
> 
> Use the global client_messaging_context. smbstatus only runs as root, so
> failing client_messaging_context() we just throw an error.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/utils/status.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)
> 
> diff --git a/source3/utils/status.c b/source3/utils/status.c
> index d04efedee3f..27f95a7522d 100644
> --- a/source3/utils/status.c
> +++ b/source3/utils/status.c
> @@ -528,7 +528,6 @@ int main(int argc, const char *argv[])
>  	};
>  	TALLOC_CTX *frame = talloc_stackframe();
>  	int ret = 0;
> -	struct tevent_context *ev;
>  	struct messaging_context *msg_ctx = NULL;
>  	char *db_path;
>  	bool ok;
> @@ -607,28 +606,9 @@ int main(int argc, const char *argv[])
>  		d_printf("using configfile = %s\n", get_dyn_CONFIGFILE());
>  	}
>  
> -	if (!lp_load_initial_only(get_dyn_CONFIGFILE())) {
> -		fprintf(stderr, "Can't load %s - run testparm to debug it\n",
> -			get_dyn_CONFIGFILE());
> -		ret = -1;
> -		goto done;
> -	}
> -
> -
> -	/*
> -	 * This implicitly initializes the global ctdbd connection,
> -	 * usable by the db_open() calls further down.
> -	 */
> -	ev = samba_tevent_context_init(NULL);
> -	if (ev == NULL) {
> -		fprintf(stderr, "samba_tevent_context_init failed\n");
> -		ret = -1;
> -		goto done;
> -	}
> -
> -	msg_ctx = messaging_init(NULL, ev);
> +	msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
>  	if (msg_ctx == NULL) {
> -		fprintf(stderr, "messaging_init failed\n");
> +		fprintf(stderr, "client_messaging_context failed, not root?\n");
>  		ret = -1;
>  		goto done;
>  	}
> -- 
> 2.13.6
> 
> 
> From fc72d0ba95a294f3b9c740bb545931e40163fc8c Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 13:47:33 +0200
> Subject: [PATCH 10/12] s3: rpcclient: use client_messaging_context
> 
> Remove global rpcclient_msg_ctx and use the new
> client_messaging_context() instead.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/rpcclient/cmd_netlogon.c |  8 +++++++-
>  source3/rpcclient/rpcclient.c    | 44 ++++++++++++++++------------------------
>  source3/rpcclient/rpcclient.h    |  1 -
>  3 files changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/source3/rpcclient/cmd_netlogon.c b/source3/rpcclient/cmd_netlogon.c
> index 8d62ef7e095..cff47fe211d 100644
> --- a/source3/rpcclient/cmd_netlogon.c
> +++ b/source3/rpcclient/cmd_netlogon.c
> @@ -564,6 +564,7 @@ static NTSTATUS cmd_netlogon_change_trust_pw(struct rpc_pipe_client *cli,
>  {
>          NTSTATUS result = NT_STATUS_UNSUCCESSFUL;
>  	const char *dcname = cli->desthost;
> +	struct messaging_context *msg_ctx = NULL;
>  
>          /* Check arguments */
>  
> @@ -572,8 +573,13 @@ static NTSTATUS cmd_netlogon_change_trust_pw(struct rpc_pipe_client *cli,
>                  return NT_STATUS_OK;
>          }
>  
> +	msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
> +	if (msg_ctx == NULL) {
> +		return NT_STATUS_INTERNAL_ERROR;
> +	}
> +
>  	result = trust_pw_change(rpcclient_netlogon_creds,
> -				 rpcclient_msg_ctx,
> +				 msg_ctx,
>  				 cli->binding_handle,
>  				 lp_workgroup(),
>  				 dcname,
> diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
> index b4e25e6e479..0e0df8bece2 100644
> --- a/source3/rpcclient/rpcclient.c
> +++ b/source3/rpcclient/rpcclient.c
> @@ -50,7 +50,6 @@ static enum dcerpc_AuthLevel pipe_default_auth_level = DCERPC_AUTH_LEVEL_NONE;
>  static unsigned int timeout = 0;
>  static enum dcerpc_transport_t default_transport = NCACN_NP;
>  
> -struct messaging_context *rpcclient_msg_ctx;
>  struct cli_state *rpcclient_cli_state;
>  struct netlogon_creds_cli_context *rpcclient_netlogon_creds;
>  static const char *rpcclient_netlogon_domain;
> @@ -711,9 +710,13 @@ static NTSTATUS do_cmd(struct cli_state *cli,
>  {
>  	NTSTATUS ntresult;
>  	WERROR wresult;
> +	struct messaging_context *msg_ctx = NULL;
> +	TALLOC_CTX *mem_ctx = NULL;
>  
> -	TALLOC_CTX *mem_ctx;
> -
> +	msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
> +	if (msg_ctx == NULL) {
> +		return NT_STATUS_INTERNAL_ERROR;
> +	}
>  	/* Create mem_ctx */
>  
>  	if (!(mem_ctx = talloc_stackframe())) {
> @@ -762,12 +765,12 @@ static NTSTATUS do_cmd(struct cli_state *cli,
>  		case DCERPC_AUTH_TYPE_SCHANNEL:
>  			TALLOC_FREE(rpcclient_netlogon_creds);
>  			ntresult = cli_rpc_pipe_open_schannel(
> -				cli, rpcclient_msg_ctx,
> +				cli, msg_ctx,
>  				cmd_entry->table,
>  				default_transport,
>  				rpcclient_netlogon_domain,
>  				&cmd_entry->rpc_pipe,
> -				rpcclient_msg_ctx,
> +				msg_ctx,
>  				&rpcclient_netlogon_creds);
>  			break;
>  		default:
> @@ -805,8 +808,8 @@ static NTSTATUS do_cmd(struct cli_state *cli,
>  
>  			ntresult = rpccli_create_netlogon_creds_ctx(creds,
>  							dc_name,
> -							rpcclient_msg_ctx,
> -							rpcclient_msg_ctx,
> +							msg_ctx,
> +							msg_ctx,
>  							&rpcclient_netlogon_creds);
>  			if (!NT_STATUS_IS_OK(ntresult)) {
>  				DEBUG(0, ("Could not initialise credentials for %s.\n",
> @@ -950,7 +953,7 @@ static NTSTATUS process_cmd(struct user_auth_info *auth_info,
>  	const char *binding_string = NULL;
>  	const char *host;
>  	int signing_state = SMB_SIGNING_IPC_DEFAULT;
> -	struct tevent_context *ev_ctx = NULL;
> +	struct messaging_context *msg_ctx = NULL;
>  
>  	/* make sure the vars that get altered (4th field) are in
>  	   a fixed location or certain compilers complain */
> @@ -1016,18 +1019,13 @@ static NTSTATUS process_cmd(struct user_auth_info *auth_info,
>  	poptFreeContext(pc);
>  	popt_burn_cmdline_password(argc, argv);
>  
> -	ev_ctx = samba_tevent_context_init(frame);
> -	if (ev_ctx == NULL) {
> -		fprintf(stderr, "Could not init event context\n");
> -		result = 1;
> -		goto done;
> -	}
> -
> -	nt_status = messaging_init_client(ev_ctx,
> -					  ev_ctx,
> -					  &rpcclient_msg_ctx);
> -	if (geteuid() != 0 &&
> -			NT_STATUS_EQUAL(nt_status, NT_STATUS_ACCESS_DENIED)) {
> +	msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
> +	if (msg_ctx == NULL) {
> +		if (geteuid() == 0) {
> +			fprintf(stderr, "Could not init messaging context\n");
> +			result = 1;
> +			goto done;
> +		}
>  		/*
>  		 * Normal to fail to initialize messaging context
>  		 * if we're not root as we don't have ability to
> @@ -1035,10 +1033,6 @@ static NTSTATUS process_cmd(struct user_auth_info *auth_info,
>  		 */
>  		DBG_NOTICE("Unable to initialize messaging context. "
>  			"Must be root to do that.\n");
> -	} else if (!NT_STATUS_IS_OK(nt_status)) {
> -		fprintf(stderr, "Could not init messaging context\n");
> -		result = 1;
> -		goto done;
>  	}
>  
>  	if (!init_names()) {
> @@ -1257,8 +1251,6 @@ static NTSTATUS process_cmd(struct user_auth_info *auth_info,
>  	}
>  	popt_free_cmdline_auth_info();
>  	netlogon_creds_cli_close_global_db();
> -	TALLOC_FREE(rpcclient_msg_ctx);
> -	TALLOC_FREE(ev_ctx);
>  	TALLOC_FREE(frame);
>  	return result;
>  }
> diff --git a/source3/rpcclient/rpcclient.h b/source3/rpcclient/rpcclient.h
> index 7697d3d0485..16febf6d145 100644
> --- a/source3/rpcclient/rpcclient.h
> +++ b/source3/rpcclient/rpcclient.h
> @@ -42,7 +42,6 @@ struct cmd_set {
>  	bool use_netlogon_creds;
>  };
>  
> -extern struct messaging_context *rpcclient_msg_ctx;
>  extern struct cli_state *rpcclient_cli_state;
>  extern struct netlogon_creds_cli_context *rpcclient_netlogon_creds;
>  
> -- 
> 2.13.6
> 
> 
> From ad65c9df8d6561bc1561c65fcedeb03ed6215c59 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 16:28:11 +0200
> Subject: [PATCH 11/12] s3: net: use client_messaging_context()
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/utils/net.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/source3/utils/net.c b/source3/utils/net.c
> index 44daa6088ca..371309c9b54 100644
> --- a/source3/utils/net.c
> +++ b/source3/utils/net.c
> @@ -915,9 +915,7 @@ static struct functable net_func[] = {
>  	const char **argv_const = discard_const_p(const char *, argv);
>  	poptContext pc;
>  	TALLOC_CTX *frame = talloc_stackframe();
> -	struct tevent_context *ev;
>  	struct net_context *c = talloc_zero(frame, struct net_context);
> -	NTSTATUS status;
>  
>  	struct poptOption long_options[] = {
>  		{"help",	'h', POPT_ARG_NONE,   0, 'h'},
> @@ -1031,30 +1029,19 @@ static struct functable net_func[] = {
>  		}
>  	}
>  
> -	if (!lp_load_initial_only(get_dyn_CONFIGFILE())) {
> -		d_fprintf(stderr, "Can't load %s - run testparm to debug it\n",
> -			  get_dyn_CONFIGFILE());
> -		exit(1);
> -	}
> -
> -	ev = samba_tevent_context_init(c);
> -	if (ev == NULL) {
> -		d_fprintf(stderr, "samba_tevent_context_init failed\n");
> -		exit(1);
> -	}
> -	status = messaging_init_client(c, ev, &c->msg_ctx);
> -	if (geteuid() != 0 &&
> -			NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
> +	c->msg_ctx = client_messaging_context(get_dyn_CONFIGFILE());
> +	if (c->msg_ctx == NULL) {
> +		if (geteuid() == 0) {
> +			fprintf(stderr, "Could not init messaging context\n");
> +			exit(1);
> +		}
>  		/*
>  		 * Normal to fail to initialize messaging context
>  		 * if we're not root as we don't have ability to
>  		 * read lock directory.
>  		 */
>  		DBG_NOTICE("Unable to initialize messaging context. "
> -			"Must be root to do that.\n");
> -	} else if (!NT_STATUS_IS_OK(status)) {
> -		d_fprintf(stderr, "Failed to init messaging context\n");
> -		exit(1);
> +			   "Must be root to do that.\n");
>  	}
>  
>  	if (!lp_load_global(get_dyn_CONFIGFILE())) {
> -- 
> 2.13.6
> 
> 
> From 5ba533c41dc50ff1dcd4877c04570d6fcfcd637a Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 10 Jul 2018 16:29:46 +0200
> Subject: [PATCH 12/12] s3: messaging: remove unused messaging_init_client()
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13465
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/include/messages.h | 3 ---
>  source3/lib/messages.c     | 9 ---------
>  2 files changed, 12 deletions(-)
> 
> diff --git a/source3/include/messages.h b/source3/include/messages.h
> index 29c394af317..f7b40664b0b 100644
> --- a/source3/include/messages.h
> +++ b/source3/include/messages.h
> @@ -46,9 +46,6 @@ struct messaging_rec;
>  
>  struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx, 
>  					 struct tevent_context *ev);
> -NTSTATUS messaging_init_client(TALLOC_CTX *mem_ctx,
> -			       struct tevent_context *ev,
> -			       struct messaging_context **pmsg_ctx);
>  
>  struct server_id messaging_server_id(const struct messaging_context *msg_ctx);
>  struct tevent_context *messaging_tevent_context(
> diff --git a/source3/lib/messages.c b/source3/lib/messages.c
> index 82a177856f6..df7af2e50f1 100644
> --- a/source3/lib/messages.c
> +++ b/source3/lib/messages.c
> @@ -624,15 +624,6 @@ struct messaging_context *messaging_init(TALLOC_CTX *mem_ctx,
>  	return ctx;
>  }
>  
> -NTSTATUS messaging_init_client(TALLOC_CTX *mem_ctx,
> -			       struct tevent_context *ev,
> -			       struct messaging_context **pmsg_ctx)
> -{
> -	return messaging_init_internal(mem_ctx,
> -					ev,
> -					pmsg_ctx);
> -}
> -
>  struct server_id messaging_server_id(const struct messaging_context *msg_ctx)
>  {
>  	return msg_ctx->id;
> -- 
> 2.13.6
> 




More information about the samba-technical mailing list