[PATCH] fix crash in smb.conf processing

Alexander Bokovoy ab at samba.org
Fri Jan 22 13:50:02 UTC 2016


Hi,

please review and push.

The bug is pretty serious and as such is blocker for FreeIPA use of
Samba in Fedora 23 -- after commit
795c543d858b2452f062a02846c2f908fe4cffe4 smb.conf processing was
improved but nobody did test it for cases when config is re-read several
times like it happens with the registry backend for smb.conf.

More details are in the commit message.

If you want to test on Fedora 23, my COPR repository
https://copr.fedorainfracloud.org/coprs/abbra/samba-test/ has pre-built
packages.

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From d8a03eeab8ece3d9001b087fc658272174f92a25 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <ab at samba.org>
Date: Fri, 22 Jan 2016 11:44:03 +0200
Subject: [PATCH] s3-parm: clean up defaults when removing global parameters

When globals are re-initialized, they are cleared and globals' talloc
context is freed. However, parm_table still contains a reference to the
global value in the defaults. This confuses lpcfg_string_free() after
commit 795c543d858b2452f062a02846c2f908fe4cffe4 because it tries to
free already freed pointer which is passed by lp_save_defaults():

....
    case P_STRING:
    case P_USTRING:
                  lpcfg_string_set(Globals.ctx,
                                   &parm_table[i].def.svalue,
                                   *(char **)lp_parm_ptr(NULL, &parm_table[i]));
....

here &parm_table[i].def.svalue is passed to lpcfg_string_free() but it
is a pointer to a value allocated with previous Globals.ctx which
already was freed.

This specifically affects registry backend of smb.conf in lp_load_ex()
where init_globals() called explicitly to re-init globals after
lp_save_defaults() if we have registry backend defined.

Signed-off-by: Alexander Bokovoy <ab at samba.org>
---
 source3/param/loadparm.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 9f4a2b4..f8ecab7 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -399,8 +399,25 @@ static void free_parameters_by_snum(int snum)
  */
 static void free_global_parameters(void)
 {
+	uint32_t i;
+	struct parm_struct *parm;
+
 	free_param_opts(&Globals.param_opt);
 	free_parameters_by_snum(GLOBAL_SECTION_SNUM);
+
+	/* Reset references in the defaults because the context is going to be freed */
+	for (i=0; parm_table[i].label; i++) {
+		parm = &parm_table[i];
+		if ((parm->type == P_STRING) ||
+		    (parm->type == P_USTRING)) {
+			if ((parm->def.svalue != NULL) &&
+			    (*(parm->def.svalue) != '\0')) {
+				if (talloc_parent(parm->def.svalue) == Globals.ctx) {
+					parm->def.svalue = NULL;
+				}
+			}
+		}
+	}
 	TALLOC_FREE(Globals.ctx);
 }
 
-- 
2.5.0



More information about the samba-technical mailing list