tevent_abort_nesting crash in idmap_ad

Volker Lendecke vl at samba.org
Sat Jun 25 15:42:04 UTC 2016


On Sat, Jun 25, 2016 at 08:16:57AM -0700, Jeremy Allison wrote:
> Oh I agree. I don't think offering such an async api
> was a great idea when under the covers it can never
> be so. But it's easily avoided here, so let's just
> do that. No large code reverts needed.

Can you please explain what exactly is avoided here? If we still offer
_send and _recv calls for bind in tldap, that's a completely pointless
thing that needs to be removed. We *might* be able to do async calls
with normal ldap traffic, but who knows what happens when a ticket
expires and we have to re-acquire it in the background. Nested event
loop again, and this time practically not reproducible, because it
will take many hours before it happens.

> The act of getting krb5 creds under both gssapi and gensec
> is sync, and will (probably) always remain sync. Not much
> we can do about it - just be aware of the issue and avoid
> it in future code.

So that makes gensec unusable in an async API, sorry. With SMB there's
not much we can do, because we have too much code depend on it. But
with TLDAP we still have the chance to do the right thing and admit
what I have tried to achieve is impossible.

> > The revert is just as simple. It also fixes the crash it just as
> > nicely.
> 
> Much bigger code change I'm afraid. Smaller changes usually win :-).

Well, I am really sorry that it was not possible to do that piece by
piece. I already felt bad about this huge commit. But I did not see
another way, and now it haunts me because your commit has less lines?

> tldap is in the code, works really well as a
> significant async improvement for all general ldap calls,

We don't know how well it works. It was never used in production, we
have no idea about its performance etc. Also, may I remind you of your
attitude towards ASN.1? This is a shitload of homegrown ASN.1 code that
others have done already for us. The old code has its flaws, but it is
known to work in production for ages. We should go back and revert the
big chainsaw commit and do the thing you are arguing for: Slowly fix
the bugs in more understandable pieces in the old code.

Volker



More information about the samba-technical mailing list