[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#6337) Combining back-relay/slapo-rwm and back-hdb/slapo-accesslog seg faults

masarati@aero.polimi.it writes:
> What seems to happen is that bind's response is sent outside the backend,
> by the frontend itself.  As a consequence, back-relay's callback
> structure, which is located on the stack, is dangling at the moment it is
> used.

I imagine existing OpenLDAP installations' "access" statements,
ldapwhoami and whatnot can break with a straightforward fix to this.
I'm unsure about this and I'm not about to explore/test now, sorry.

That is, I imagine making the back-relay callback structure persist
after relay_back_op_bind() at times can change when/whether a Bind
succeeds, and what the bound DN will be after the Bind (compared to
before the fix).

That doesn't seem like a change to make in the middle of RE24.  Instead
I suggest to default to the old behavior: The callback structure will
do nothing after relay_back_op_bind().

We can add an option to persist the callback structure so that
back-relay will actually work for Bind:-)

To make the structure do nothing, it could get a "do nothing" flag which
relay_back_op() sets on the way out if it doesn't find and remove the
callback.  Alternatively, relay_back_op() can search for the callback
instead of just checking the first callback in op->o_callback.

Some other points:

- Is it enough to just persist the callback structure after the call?
  The other code relies on the caller to modify/restore op->o_bd,
  but now there'll be no caller which will be doing this.

  I have no idea without single-stepping this - between callbacks,
  back-relay and overlays I'm losing track of what is happening when.

- Extended operations should get the same fix, since bi->bi_extended
  also relies on the frontend to send the response.

  Which means, I suppose, that instead of just testing 'which ==
  op_bind' there could be a "persist" flag bit in relay_fail_modes[] -
  renamed relay_modes[] since they are now about more than failure.

- Same with bi->bi_chk_referrals, except it is not implemented.  There
  is a comment that relay_back_chk_referrals is unfixable, but I don't
  know if that's related to this.

(BTW, I hope back-relay/op.c is still readable?  I tried to comment it
reasonably when making it table-driven, but it's always easier to read
my own code than someone else's...)