From Günther Deschner: I think libsmbclient is really doing something wrong here: in smbc_free_context libsmbclient just calls free() on the string options so it assumes the callers have malloced them before setting them via smbc_set calls. From Jeremy Allison: libsmbclient should have been deep copying the strings from the start, not just stealing the pointers. Can we track down the gvfs backend code that calls this and see how hard it would be to fix this mess ? Ok, that latest git code I can find for gvfs both say: http://git.gnome.org/browse/gvfs/tree/daemon/gvfsbackendsmb.c http://git.gnome.org/browse/gvfs/tree/daemon/gvfsbackendsmbbrowse.c /* FIXME: is strdup() still needed here? -- removed */ if (default_workgroup != NULL) smbc_setWorkgroup (smb_context, default_workgroup); So it looks like we really need to make this a memory duplicating interface asap and fix the calling code in smbc_init_context().
Created attachment 5191 [details] Patch to clean up these interfaces to be malloc-safe. Please review this fix for master. If you concur Guenther and can confirm it's safe for gvfs I'll back-port for 3.5.0, 3.4.5 and 3.3.11. Jeremy.
Comment on attachment 5191 [details] Patch to clean up these interfaces to be malloc-safe. Adding Derrell to the review list.
Created attachment 5192 [details] Complete patch for master. Complete patch, includes the fixes that went into master as : 2d41b1ab78639abe4ae030ff482573f464564dd7 and f85b6ee90b88c7f7b2a92c8a5f3e2ebe59c1087b
Created attachment 5193 [details] git-am format patch for 3.5.0.
Created attachment 5194 [details] git-am format patch for 3.4.5.
Created attachment 5195 [details] git-am fix for 3.3.11.
Looks entirely right. Karolin, please put this into the relevant branches. Thanks, Volker
BTW, while this is definitely the right thing to do, it might create memleaks in correct current users of the current API. Or did I get something wrong? Volker
The only current user of the API I could find is Gnome gvfs, which explicitly *removed* the strdup they had in older code. So right now they'll probably end up with a crash, and I'm ok with memleaking "workgroup", "netbiosname" and "username" in older code which strdup'ed it, for the sake of ending up with a sane interface. No other way to fix it. Please push to all branches. Jeremy.
These changes look fine to me. Derrell
Pushed to all branches. Closing out bug report. Thanks!