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?