[PATCH] Fix configuration reload in s3/loadparm
Volker Lendecke
Volker.Lendecke at SerNet.DE
Wed Nov 22 13:31:33 UTC 2017
Cancelled the autobuild.
Volker
On Wed, Nov 22, 2017 at 02:17:05PM +0100, Swen Schillig wrote:
> 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
>
--
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
More information about the samba-technical
mailing list