s4: server: Fix crash in NTVFS server caused by ordering of destructor calls.

In the NTVFS server we have the following talloc heirarchy:

                                   event_ctx
                                     |
        ---------------------------------------------------- .. other children
        |                       |                      |
     msg_dgm_ref              srv_conn            msg_dgm_ref
        ^                       |
        |                    NTVFS structures
        |                       |
        |                    XXXXXX
        |                       |
        |                       |
        --------------------- pointer to msg_dgm_ref

Some of the structures under NTVFS （marked XXXXX) can have
pointers to imessaging contexts which internally have pointers
to msg_dgm_ref structurs allocated off event_ctx.

The original code calls:

        model_ops->terminate(event_ctx, srv_conn->lp_ctx, reason);
        talloc_free(srv_conn);

But model_ops->terminate() calls talloc_free(event_ctx) and
then calls exit(). In this case srv_conn is never explicitly
freed, but only freed as a talloc child of the event_ctx.

Depending on the ordering of the linked list of talloc children
under event_ctx（which can be reordered via talloc_free/reinit
of msg_dgm_ref) a pointer to msg_dgm_ref under srv_conn can
be left pointing to memory that was already freed. This pointer
is then used in the destructor for a file object called when
srv_conn is freed.

Re-ordering this to explicitly call TALLOC_FREE(srv_conn) first
and then model_ops->terminate() fixes this problem.

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Böhme <slow@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Sun Apr  2 05:18:39 CEST 2017 on sn-devel-144

diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c
index 29e6515622b1e069e9264660be655a540da743db..bda28ad26f8577d84bcb7475ea2d07fcaa4e6235 100644 (file)
--- a/source4/smbd/service_stream.c
+++ b/source4/smbd/service_stream.c
@@ -54,6 +54,7 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char
 {
        struct tevent_context *event_ctx = srv_conn->event.ctx;
        const struct model_ops *model_ops = srv_conn->model_ops;
+       struct loadparm_context *lp_ctx = srv_conn->lp_ctx;
 
        if (!reason) reason = "unknown reason";
 
@@ -79,8 +80,8 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char
        talloc_free(srv_conn->event.fde);
        srv_conn->event.fde = NULL;
        imessaging_cleanup(srv_conn->msg_ctx);
-       model_ops->terminate(event_ctx, srv_conn->lp_ctx, reason);
-       talloc_free(srv_conn);
+       TALLOC_FREE(srv_conn);
+       model_ops->terminate(event_ctx, lp_ctx, reason);
 }
 
 /**