charset: fixed a problem with the global use of the iconv_convenience structure

We had a crash bug where a cached copy of a iconv convenience pointer
was used after being freed when loadparm asked for iconv to
reload. This could happen if a python module used a iconv based
function before loadparm was completed.

The fix is to ensure that any use of this pointer remains valid, by
reusing the pointer itself when it has already been initialised, but
filling in the child elements with the updated values.

diff --git a/lib/util/charset/charcnv.c b/lib/util/charset/charcnv.c
index a479f4442653eb4aa9907d7047d53987cd30ed04..efdb3ed2b33bb532c70b91eebb5e5a939524412e 100644 (file)
--- a/lib/util/charset/charcnv.c
+++ b/lib/util/charset/charcnv.c
@@ -39,6 +39,7 @@
  */
 
 struct smb_iconv_convenience {
+       TALLOC_CTX *child_ctx;
        const char *unix_charset;
        const char *dos_charset;
        bool native_iconv;
@@ -83,22 +84,45 @@ static int close_iconv_convenience(struct smb_iconv_convenience *data)
        return 0;
 }
 
-_PUBLIC_ struct smb_iconv_convenience *smb_iconv_convenience_init(TALLOC_CTX *mem_ctx,
-                                                        const char *dos_charset,
-                                                        const char *unix_charset,
-                                                        bool native_iconv)
+/*
+  the old_ic is passed in here as the smb_iconv_convenience structure
+  is used as a global pointer in some places (eg. python modules). We
+  don't want to invalidate those global pointers, but we do want to
+  update them with the right charset information when loadparm
+  runs. To do that we need to re-use the structure pointer, but
+  re-fill the elements in the structure with the updated values
+ */
+_PUBLIC_ struct smb_iconv_convenience *smb_iconv_convenience_reinit(TALLOC_CTX *mem_ctx,
+                                                                   const char *dos_charset,
+                                                                   const char *unix_charset,
+                                                                   bool native_iconv,
+                                                                   struct smb_iconv_convenience *old_ic)
 {
-       struct smb_iconv_convenience *ret = talloc_zero(mem_ctx, 
-                                       struct smb_iconv_convenience);
+       struct smb_iconv_convenience *ret;
 
+       if (old_ic != NULL) {
+               ret = old_ic;
+               close_iconv_convenience(ret);
+               talloc_free(ret->child_ctx);
+               ZERO_STRUCTP(ret);
+       } else {
+               ret = talloc_zero(mem_ctx, struct smb_iconv_convenience);
+       }
        if (ret == NULL) {
                return NULL;
        }
 
+       /* we use a child context to allow us to free all ptrs without
+          freeing the structure itself */
+       ret->child_ctx = talloc_new(ret);
+       if (ret->child_ctx == NULL) {
+               return NULL;
+       }
+
        talloc_set_destructor(ret, close_iconv_convenience);
 
-       ret->dos_charset = talloc_strdup(ret, dos_charset);
-       ret->unix_charset = talloc_strdup(ret, unix_charset);
+       ret->dos_charset = talloc_strdup(ret->child_ctx, dos_charset);
+       ret->unix_charset = talloc_strdup(ret->child_ctx, unix_charset);
        ret->native_iconv = native_iconv;
 
        return ret;

diff --git a/lib/util/charset/charset.h b/lib/util/charset/charset.h
index 2c8aa41ad5c78287c4ce627b4664dfaacfbef0b9..cc57b3eb542e9bf9e00450d5f52c13bf0df80022 100644 (file)
--- a/lib/util/charset/charset.h
+++ b/lib/util/charset/charset.h
@@ -163,10 +163,11 @@ codepoint_t tolower_m(codepoint_t val);
 int codepoint_cmpi(codepoint_t c1, codepoint_t c2);
 
 /* Iconv convenience functions */
-struct smb_iconv_convenience *smb_iconv_convenience_init(TALLOC_CTX *mem_ctx,
-                                                        const char *dos_charset,
-                                                        const char *unix_charset,
-                                                        bool native_iconv);
+struct smb_iconv_convenience *smb_iconv_convenience_reinit(TALLOC_CTX *mem_ctx,
+                                                          const char *dos_charset,
+                                                          const char *unix_charset,
+                                                          bool native_iconv,
+                                                          struct smb_iconv_convenience *old_ic);
 
 bool convert_string_convenience(struct smb_iconv_convenience *ic,
                                charset_t from, charset_t to,

diff --git a/lib/util/charset/util_unistr.c b/lib/util/charset/util_unistr.c
index f8207261c5fbb9fbd04359c318d3f8381a2cb651..520ce054680ed1b43cb14b312db23602da60fd58 100644 (file)
--- a/lib/util/charset/util_unistr.c
+++ b/lib/util/charset/util_unistr.c
@@ -26,7 +26,8 @@ struct smb_iconv_convenience *global_iconv_convenience = NULL;
 static inline struct smb_iconv_convenience *get_iconv_convenience(void)
 {
        if (global_iconv_convenience == NULL)
-               global_iconv_convenience = smb_iconv_convenience_init(talloc_autofree_context(), "ASCII", "UTF-8", true);
+               global_iconv_convenience = smb_iconv_convenience_reinit(talloc_autofree_context(),
+                                                                       "ASCII", "UTF-8", true, NULL);
        return global_iconv_convenience;
 }
 

diff --git a/librpc/ndr/ndr.c b/librpc/ndr/ndr.c
index 4d763e0eadd9743ab84ab275f22e198c4366ec65..90be787526f293456e1c36ebc36396a1d93b396f 100644 (file)
--- a/librpc/ndr/ndr.c
+++ b/librpc/ndr/ndr.c
@@ -260,7 +260,8 @@ _PUBLIC_ void ndr_print_function_debug(ndr_print_function_t fn, const char *name
         * this all the way down 
         */
 #if _SAMBA_BUILD_ == 4
-       ndr->iconv_convenience = smb_iconv_convenience_init(talloc_autofree_context(), "ASCII", "UTF-8", true);
+       ndr->iconv_convenience = smb_iconv_convenience_reinit(talloc_autofree_context(),
+                                                             "ASCII", "UTF-8", true, NULL);
 #endif
 
        fn(ndr, name, flags, ptr);
@@ -289,7 +290,8 @@ _PUBLIC_ char *ndr_print_struct_string(TALLOC_CTX *mem_ctx, ndr_print_fn_t fn, c
         * this all the way down 
         */
 #if _SAMBA_BUILD_ == 4
-       ndr->iconv_convenience = smb_iconv_convenience_init(talloc_autofree_context(), "ASCII", "UTF-8", true);
+       ndr->iconv_convenience = smb_iconv_convenience_reinit(talloc_autofree_context(),
+                                                             "ASCII", "UTF-8", true, NULL);
 #endif
 
        fn(ndr, name, ptr);

diff --git a/pidl/tests/ndr_string.pl b/pidl/tests/ndr_string.pl
index 7b76c7b29508fae1db12aab32939ade7b1e92d15..e00dd01c8e26a63119828116f1fe453094855123 100755 (executable)
--- a/pidl/tests/ndr_string.pl
+++ b/pidl/tests/ndr_string.pl
@@ -15,7 +15,7 @@ test_samba4_ndr("string-pull-empty",
        uint8_t data[] = { 0x00, 0x00, 0x00, 0x00 };
        DATA_BLOB b = { data, 4 };
        struct ndr_pull *ndr = ndr_pull_init_blob(&b, NULL, 
-               smb_iconv_convenience_init(NULL, "ASCII", "UTF8", true));
+               smb_iconv_convenience_reinit(NULL, "ASCII", "UTF8", true, NULL));
        struct TestString r;
        r.in.data = NULL;
 
@@ -38,7 +38,7 @@ test_samba4_ndr("string-ascii-pull",
                                           \'f\', \'o\', \'o\', 0 };
        DATA_BLOB b = { data, 8 };
        struct ndr_pull *ndr = ndr_pull_init_blob(&b, NULL,
-               smb_iconv_convenience_init(NULL, "ASCII", "UTF8", true));
+               smb_iconv_convenience_reinit(NULL, "ASCII", "UTF8", true, NULL));
        struct TestString r;
        r.in.data = NULL;
 
@@ -75,7 +75,7 @@ test_samba4_ndr("string-wchar-fixed-array-01",
        };
        DATA_BLOB b = { data, sizeof(data) };
        struct ndr_pull *ndr = ndr_pull_init_blob(&b, NULL,
-               smb_iconv_convenience_init(NULL, "ASCII", "UTF8", true));
+               smb_iconv_convenience_reinit(NULL, "ASCII", "UTF8", true, NULL));
        struct TestString r;
        struct TestStringStruct str;
        r.in.str = &str;
@@ -121,7 +121,7 @@ test_samba4_ndr("string-wchar-fixed-array-02",
        };
        DATA_BLOB b = { data, sizeof(data) };
        struct ndr_pull *ndr = ndr_pull_init_blob(&b, NULL,
-               smb_iconv_convenience_init(NULL, "ASCII", "UTF8", true));
+               smb_iconv_convenience_reinit(NULL, "ASCII", "UTF8", true, NULL));
        struct TestString r;
        struct TestStringStruct str;
        r.in.str = &str;
@@ -153,7 +153,7 @@ test_samba4_ndr("string-wchar-fixed-array-03",
        };
        DATA_BLOB b = { data, sizeof(data) };
        struct ndr_pull *ndr = ndr_pull_init_blob(&b, NULL,
-               smb_iconv_convenience_init(NULL, "ASCII", "UTF8", true));
+               smb_iconv_convenience_reinit(NULL, "ASCII", "UTF8", true, NULL));
        struct TestString r;
        struct TestStringStruct str;
        r.in.str = &str;
@@ -175,7 +175,7 @@ test_samba4_ndr("string-out",
                                           \'f\', \'o\', \'o\', 0 };
        DATA_BLOB b = { data, 8 };
        struct ndr_pull *ndr = ndr_pull_init_blob(&b, NULL,
-               smb_iconv_convenience_init(NULL, "ASCII", "UTF8", true));
+               smb_iconv_convenience_reinit(NULL, "ASCII", "UTF8", true, NULL));
        struct TestString r;
        char *str = NULL;
        r.out.data = &str;

diff --git a/source4/param/loadparm.c b/source4/param/loadparm.c
index 46bbceb8620606dbfac70481b9a3402225bae0c3..2bee0fbd861968456b7fad7d45cab7acc623f1ab 100644 (file)
--- a/source4/param/loadparm.c
+++ b/source4/param/loadparm.c
@@ -2692,8 +2692,8 @@ struct smb_iconv_convenience *lp_iconv_convenience(struct loadparm_context *lp_c
        if (lp_ctx == NULL) {
                static struct smb_iconv_convenience *fallback_ic = NULL;
                if (fallback_ic == NULL)
-                       fallback_ic = smb_iconv_convenience_init(talloc_autofree_context(), 
-                                                 "CP850", "UTF8", true);
+                       fallback_ic = smb_iconv_convenience_reinit(talloc_autofree_context(),
+                                                                  "CP850", "UTF8", true, NULL);
                return fallback_ic;
        }
        return lp_ctx->iconv_convenience;
@@ -2701,8 +2701,12 @@ struct smb_iconv_convenience *lp_iconv_convenience(struct loadparm_context *lp_c
 
 _PUBLIC_ void reload_charcnv(struct loadparm_context *lp_ctx)
 {
-       talloc_unlink(lp_ctx, lp_ctx->iconv_convenience);
-       global_iconv_convenience = lp_ctx->iconv_convenience = smb_iconv_convenience_init_lp(lp_ctx, lp_ctx);
+       struct smb_iconv_convenience *old_ic = lp_ctx->iconv_convenience;
+       if (old_ic == NULL) {
+               old_ic = global_iconv_convenience;
+       }
+       lp_ctx->iconv_convenience = smb_iconv_convenience_reinit_lp(lp_ctx, lp_ctx, old_ic);
+       global_iconv_convenience = lp_ctx->iconv_convenience;
 }
 
 void lp_smbcli_options(struct loadparm_context *lp_ctx,

diff --git a/source4/param/param.h b/source4/param/param.h
index f72fa76174a953c2be655fd63828053d1116cd81..5435941f2be1e399c8b0017b1526cbbb794e74ba 100644 (file)
--- a/source4/param/param.h
+++ b/source4/param/param.h
@@ -438,8 +438,9 @@ bool run_init_functions(init_module_fn *fns);
 init_module_fn *load_samba_modules(TALLOC_CTX *mem_ctx, struct loadparm_context *lp_ctx, const char *subsystem);
 const char *lp_messaging_path(TALLOC_CTX *mem_ctx, 
                                       struct loadparm_context *lp_ctx);
-struct smb_iconv_convenience *smb_iconv_convenience_init_lp(TALLOC_CTX *mem_ctx,
-                                                        struct loadparm_context *lp_ctx);
+struct smb_iconv_convenience *smb_iconv_convenience_reinit_lp(TALLOC_CTX *mem_ctx,
+                                                             struct loadparm_context *lp_ctx,
+                                                             struct smb_iconv_convenience *old_ic);
 
 const char *lp_sam_name(struct loadparm_context *lp_ctx);
 

diff --git a/source4/param/util.c b/source4/param/util.c
index d9d4eb5e2b4009345b0f1a5392cffc5bf7e4710a..bbe4c8729383780265fa1251a982b3fc024e83f2 100644 (file)
--- a/source4/param/util.c
+++ b/source4/param/util.c
@@ -299,12 +299,14 @@ const char *lp_messaging_path(TALLOC_CTX *mem_ctx,
        return smbd_tmp_path(mem_ctx, lp_ctx, "messaging");
 }
 
-struct smb_iconv_convenience *smb_iconv_convenience_init_lp(TALLOC_CTX *mem_ctx,
-                                                        struct loadparm_context *lp_ctx)
+struct smb_iconv_convenience *smb_iconv_convenience_reinit_lp(TALLOC_CTX *mem_ctx,
+                                                             struct loadparm_context *lp_ctx,
+                                                             struct smb_iconv_convenience *old_ic)
 {
-       return smb_iconv_convenience_init(mem_ctx, lp_dos_charset(lp_ctx),
-                                         lp_unix_charset(lp_ctx),
-               lp_parm_bool(lp_ctx, NULL, "iconv", "native", true));
+       return smb_iconv_convenience_reinit(mem_ctx, lp_dos_charset(lp_ctx),
+                                           lp_unix_charset(lp_ctx),
+                                           lp_parm_bool(lp_ctx, NULL, "iconv", "native", true),
+                                           old_ic);
 }
 
 

diff --git a/source4/scripting/python/modules.h b/source4/scripting/python/modules.h
index 4d1067cdd44e27a80a250d8ea73b8f5d58f4276d..c73cfff19c03a5e4e53de31e62ac67ccb9afd8a8 100644 (file)
--- a/source4/scripting/python/modules.h
+++ b/source4/scripting/python/modules.h
@@ -23,6 +23,7 @@
 void py_load_samba_modules(void);
 bool py_update_path(const char *bindir);
 
-#define py_iconv_convenience(mem_ctx) smb_iconv_convenience_init(mem_ctx, "ASCII", PyUnicode_GetDefaultEncoding(), true)
+#define py_iconv_convenience(mem_ctx) smb_iconv_convenience_reinit(mem_ctx, "ASCII",  \
+                                                                  PyUnicode_GetDefaultEncoding(), true, NULL)
 
 #endif /* __SAMBA_PYTHON_MODULES_H__ */