net-ads-search crashes when tokengroups attribute is requested

[Isaac Boukris]

On Thu, Jan 25, 2018 at 6:49 PM, Jeremy Allison <jra at samba.org> wrote:
> On Thu, Jan 25, 2018 at 03:22:02PM +0200, Isaac Boukris wrote:
>> On Thu, Jan 25, 2018 at 12:19 PM, Volker Lendecke
>> <Volker.Lendecke at sernet.de> wrote:
>> > On Wed, Jan 24, 2018 at 02:16:53PM -0800, Jeremy Allison wrote:
>> >> OK, how about this fix instead ? It keeps the ads_destroy() call
>> >> after the reconnection failure, but uses the horrific ads->is_mine
>> >> flag to ensure the ADS_STRUCT isn't thrown away causing the
>> >> valgrind failures and crashes.
>> >>
>> >> If any of the callers were indirecting into any of the
>> >> ADS_STRUCT members after a ads_do_search_retry_internal()
>> >> reconnect failure, they were already crashing (their
>> >> ads pointer had already been freed). And if they were
>> >> not, but only calling ads_destroy() on error (which
>> >> is the case for all of the code I've seen), then this
>> >> fix makes it safe for them to do so whilst still doing
>> >> the internal structure memory free that winbindd may
>> >> be depending on.
>> >>
>> >> I safe off the original value of the horrific ads->is_mine
>> >> flag and restore it afterwards, so this should have no
>> >> effect on the winbindd code that actually uses this.
>> >>
>> >> Isaac, can you test this fix to see if it also fixes
>> >> the valgrind errors ?
>> >
>> > RB+. Lets wait the confirmation before pushing though.
>>
>> Yes, it is fine, it solved the valgrind errors (same as removing ads_destroy).
>>
>> I think the attached patch is a bit cleaner though.
>
> Not quite. The ads_cleanup() call should take a
> ADS_STRUCT *, not ADS_STRUCT **. There's no need
> for it to have access to the containing pointer
> as it never frees it.
>
> I'm going to push my existing patch, you can
> propose yours (when fixed:-) as a cleanup
> on top.


As we all seem to agree that this is_mine thingy should go away
eventually, I think your existing patch is just fine.

Thank you for your work on this!