[PATCHES] add some safeguards in idmap_hash module

Jeremy Allison jra at samba.org
Wed Mar 16 04:37:29 UTC 2016


On Tue, Mar 15, 2016 at 12:59:03PM +0100, Günther Deschner wrote:
> Hi,
> 
> the idmap_hash module can behave pretty badly in more advanced idmap
> configurations, add some more safety when using this module.
> 
> Resolves https://bugzilla.samba.org/show_bug.cgi?id=11786.
> 
> Please review and push,

In this patch:

Subject: [PATCH 3/5] s3:winbindd:idmap: check loadparm in
 domain_has_idmap_config() helper as well.

You have:

@@ -123,6 +123,9 @@ static bool idmap_init(void)
 bool domain_has_idmap_config(const char *domname)
 {
        int i;
+       const char *config_option;
+       const char *range = NULL;
+       const char *backend = NULL;
 
        idmap_init();
 
@@ -132,6 +135,23 @@ bool domain_has_idmap_config(const char *domname)
                }
        }
 
+       /* fallback: also check loadparm */
+
+       config_option = talloc_asprintf(talloc_tos(), "idmap config %s",
+                                       domname);
+       if (config_option == NULL) {
+               DEBUG(0, ("out of memory\n"));
+               return false;
+       }
+
+       range = lp_parm_const_string(-1, config_option, "range", NULL);
+       backend = lp_parm_const_string(-1, config_option, "backend", NULL);
+       if (range != NULL && backend != NULL) {
+               DEBUG(5, ("idmap configuration specified for domain '%s'\n",
+                       domname));
+               return true;
+       }
+
        return false;

This leaves config_option allocated on talloc_tos() on
exit of the function. It won't leak memory (because, talloc :-),
but it's untidy. Can you make config option a char * not a
const char * and talloc_free on all exit paths please ?

Other than that, looks great !

Cheers,

	Jeremy.




More information about the samba-technical mailing list