Segfault in smbd: messaging_dgm_fde_active

[Jeremy Allison]

On Mon, Jul 09, 2018 at 04:00:00PM -0700, Jeremy Allison via samba-technical wrote:
> On Mon, Jul 09, 2018 at 02:06:05PM +0200, Stefan Metzmacher via samba-technical wrote:
> > 
> > Please review and push:-)
> 
> OK, I'm reviewing this now, and I think I understand
> it, but once I'm done I might ask you to add some
> clarification to the commit message (or not, depending
> on how easy this is to understand :-).

OK, I do think I understand it, and I think the patch hunk
in the destructor isn't quite right.

Shoudn't it look like:

@@ -150,6 +150,11 @@ static int msg_dgm_ref_destructor(struct msg_dgm_ref *r)
        if (refs == NULL) {
                abort();
        }
+
+       if (r == next_ref) {
+               next_ref = r->next;
+       }
+
        DLIST_REMOVE(refs, r);
 
        TALLOC_FREE(r->fde);

instead of:

@ -152,6 +153,10 @@ static int msg_dgm_ref_destructor(struct msg_dgm_ref *r)
        }
        DLIST_REMOVE(refs, r);
 
+       if (r == next_ref) {
+               next_ref = NULL;
+       }
+
        TALLOC_FREE(r->fde);
 
        DBG_DEBUG("refs=%p\n", refs);

i.e. Rather than setting arbitrarily to NULL, make next_ref
point to the 'r->next' pointer of the element currently being
removed *before* we remove it. That way we don't prematurely
terminate the list walk in msg_dgm_ref_recv().

If I'm wrong, can you explain why (because then I
didn't understand it :-) ?

Thanks,

	Jeremy.