[PATCH] Fix configuration reload in s3/loadparm

[Swen Schillig]

On Wed, 2017-11-22 at 13:21 +0100, Volker Lendecke via samba-technical
wrote:
> On Wed, Nov 22, 2017 at 12:46:03PM +0100, Ralph Böhme via samba-
> technical wrote:
> > Attached patchset fixes reloading config in s3/loadparm. This
> > popped up when
> > a user went through the following config changes:
> > 
> > 1. Set [global] smb encrypt = required
> > 2. Restart Samba
> > 3. Remove the line smb encrypt = required
> > 4. smbcontrol smbd reload-config
> > 
> > Expected result: unencryptd access allowed.
> > Actual result: unencryptd access denied.
> > 
> > Bug: https://bugzilla.samba.org/show_bug.cgi?id=13051
> > 
> > Took me some time to figure out, that this happens because in
> > s3/loadparm we
> > have the defaults in the sDefault struct and any service option
> > appearing in the
> > global section modifies it. As we have no copy of the original
> > value, if the
> > user removes the config setting and reloads, the original value
> > can't be
> > restored and will still be what was initially set. *ouch*
> > 
> > It looks like we have the same problem in the toplevel loadparm,
> > but afacit all
> > the stuff that uses it doesn't support configuration reload, so I'm
> > leaving that
> > as is.
> > 
> > Please review & push if happy. Thanks!
> 
> Happy. Pushed. Thanks, Volker
> 
Volker, Ralph

I'm new to the project so please excuse me if the following is total rubbish.
I was wondering if the code shouldn't talloc_free(lp_ctx) in case of an error for lp_ctx->sDefault = taloc_zero(...)
So like this

+	lp_ctx->sDefault = talloc_zero(lp_ctx, struct loadparm_service);
+	if (lp_ctx->sDefault == NULL) {
+		DBG_ERR("talloc_zero failed\n");
+		TALLOC_FREE(lp_ctx);
+		return NULL;
+	}

I'm not concerned about the first 2 caller lp_do_parameter(), lp_set_cmdline()
where TALLOC_FREE() is called if NULL is returned by setup_lp_context(),
but I'm not really sure about the next two and especially about dump_a_parameter()
where the memory for lp_ctx might sit there for ages.

Cheers Swen