[PATCH] Fix bug 13465

[Christof Schmitt]

On Wed, Jul 11, 2018 at 11:46:41AM -0700, Christof Schmitt via samba-technical wrote:
> On Wed, Jul 11, 2018 at 03:04:27PM +0200, Ralph Böhme via samba-technical wrote:
> > On Wed, Jul 11, 2018 at 02:35:58PM +0200, Ralph Böhme wrote:
> > >On Wed, Jul 11, 2018 at 08:24:16AM +0200, Ralph Böhme wrote:
> > >>Looking...
> > >
> > >gna, the problem is starting any tool from the build directory *as
> > >root* fails if the install directory doesn not exits, eg
> > >
> > ># bin/smbd -b | grep LOCKDIR   LOCKDIR: /opt/samba/var/lock
> > ># mv /opt/samba /opt/samba.bak # bin/smbd -b | grep LOCKDIR Unable
> > >to initialize messaging context. # exit
> > >
> > >But it works as a normal user where a failing messaging init is not treated as a
> > >fatal error:
> > >
> > >$ bin/smbd -b Unable to initialize messaging context.   LOCKDIR:
> > >/opt/samba/var/lock
> 
> I see the same behavior here. This is caused by the call to:
> client_messaging_context -> lp_load_initial_only -> lp_load_ex -> pm_process -> fopen
> fopen fails because it tried to open /usr/local/samba/etc/smb.conf which
> does not exist. I assume your build refers to /opt/samba/ instead of
> /usr/local/samba/
> 
> > >Guess we should only call client_messaging_context() in
> > >popt_common_credentials_callback(), but not in
> > >popt_common_callback(). This ensures all tools that use that use
> > >POPT_COMMON_SAMBA get an automatic messaging context when needed
> > >in popt for the lp_load_client() call.
> > >
> > >popt_common_callback() will only call client_messaging_context()
> > >if (another new function) popt_common_set_init_messaging() has
> > >been called by the tool before calling poptGetContext().
> > >
> > >So any tool that doesn't use POPT_COMMON_SAMBA but uses
> > >lp_load_something > lp_load_initial_only would just call
> > >popt_common_set_init_messaging() before poptGetContext().
> > 
> > otoh, some tools like testparm don't need a msg_ctx themselves, it' just that
> > loadparm implicitly needs it with include=registry.
> > 
> > So maybe in order to give tools control over this, so they can decide how to
> > treat a failing client_messaging_context() we should just leave it up to the
> > tools to call client_messaging_context() and not call it in
> > popt_common_callback() at all. Only in popt_common_credentials_callback() as
> > needs it for lp_load_client().
> 
> Let me see if i understand this:
> 
> Keeping the call to client_messaging_context in
> popt_common_credentials_callback would be fine as
> POPT_COMMON_CREDENTIALS is only used in client tools, not in smbd.
> 
> The the patter would be to do the popt parsing first, and then call
> client_messaging_context afterwards?
> 
> I can look into making that change.

So the problem is that we do not have a valid config file at this point.
The change in behavior would be that even 'smbd -b' would require a
valid config file to get past the popt POST callback. Most parts of
Samba already require a config file, so that should be no change. If
'smbd -b' is the only part that is affected, we could also just fix
that (see attached patch).

Christof
-------------- next part --------------
From 34793a1039e7cf4e2e9f75abe2218e8c8fc08903 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 11 Jul 2018 13:25:33 -0700
Subject: [PATCH] smbd: Print build options without going through popt POST
 callback

In the case of running smbd -b without a valid config file, the
message context initialization from the popt callback would result in a
failure without printing the build options. As printing the build
options through -b should always work, call the print function directly
from the option parsing.

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

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/smbd/server.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index b73ac2b..8307920 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -1568,7 +1568,6 @@ extern void build_options(bool screen);
 	char *profile_level = NULL;
 	int opt;
 	poptContext pc;
-	bool print_build_options = False;
 	struct server_id main_server_id = {0};
         enum {
 		OPT_DAEMON = 1000,
@@ -1656,8 +1655,8 @@ extern void build_options(bool screen);
 			log_stdout = true;
 			break;
 		case 'b':
-			print_build_options = True;
-			break;
+			build_options(true);
+			exit(0);
 		default:
 			d_fprintf(stderr, "\nInvalid option %s: %s\n\n",
 				  poptBadOption(pc, 0), poptStrerror(opt));
@@ -1678,11 +1677,6 @@ extern void build_options(bool screen);
 		setup_logging(argv[0], DEBUG_FILE);
 	}
 
-	if (print_build_options) {
-		build_options(True); /* Display output to screen as well as debug */
-		exit(0);
-	}
-
 #ifdef HAVE_SETLUID
 	/* needed for SecureWare on SCO */
 	setluid(0);
-- 
1.8.3.1