On (09/03/16 18:36), Jakub Hrozek wrote: >On Wed, Mar 09, 2016 at 05:37:12PM +0100, Lukas Slebodnik wrote: >> On (09/03/16 15:21), Jakub Hrozek wrote: >> >On Wed, Mar 09, 2016 at 11:32:39AM +0100, Pavel Březina wrote: >> >> On 02/26/2016 02:03 PM, Jakub Hrozek wrote: >> >> >On Fri, Feb 26, 2016 at 11:08:45AM +0100, Pavel Březina wrote: >> >> >>On 02/24/2016 03:19 PM, Jakub Hrozek wrote: >> >> >>>Hi, >> >> >>> >> >> >>>the attached patch fixes: >> >> >>> https://fedorahosted.org/sssd/ticket/2959 >> >> >>> >> >> >>>It was confirmed by the original reporter. The bug was there since >> >> >>>2009, >> >> >>>by the way, I'm really suprised we only caught it now.. >> >> >> >> >> >>Good job finding this. I'm inclined to ack this patch, I have one >> >> >>question >> >> >>though: >> >> >> >> >> >>> if (is_user && diff[0]) { >> >> >>> /* file memberuid removal operations */ >> >> >>> name = ldb_msg_find_attr_as_string(delop->entry, DB_NAME, >> >> >>> NULL); >> >> >>> if (!name) { >> >> >>> return LDB_ERR_OPERATIONS_ERROR; >> >> >>> } >> >> >>> >> >> >>> for (i = 0; diff[i]; i++) { >> >> >>> ret = mbof_append_muop(del_ctx, &del_ctx->muops, >> >> >>> &del_ctx->num_muops, >> >> >>> LDB_FLAG_MOD_DELETE, >> >> >>> diff[i], name, >> >> >>> DB_MEMBERUID); >> >> >> >> >> >>This invokes assignment: del_ctx->muops[j] = diff[i]. To be thorough we >> >> >>should talloc_steal(del_ctx->muops, diff[i]) here to ensure proper >> >> >>talloc >> >> >>hierarchy. But it is not necessary since delctx exists only for one >> >> >>operation... what do you think? >> >> > >> >> >Yes, I think it makes sense. >> >> >> >> Ack. >> > >> >* master: 2d84b65383f2d13d6f94ac561ad92907b59062f3 >> >* sssd-1-13: cd7a272fb361626a45d54cd45daaab4bfe7ad93f >> It would be good to write unit test for this leak. >> Do you have an idea how to reproduce the leak? > >OK, I will try to write a test as part of the sysdb tests. But IIRC we >currently use a suppression file that would suppress anything from >talloc..do you know if it's possible to suppress anything from talloc >except XYZ? I was thinking about checking with leak_check_setup.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
