diff options
| author | Jeremy Allison <jra@samba.org> | 2017-04-01 15:34:48 +0000 |
|---|---|---|
| committer | Jeremy Allison <jra@samba.org> | 2017-04-02 05:18:39 +0200 |
| commit | a4599b3011647c06be0d5a914bc4f72d490f6bc5 (patch) | |
| tree | 6b4193e82b27a0be67341c0476db7d775dcfe7bb /source4/smbd | |
| parent | 8b32fc4006ae338ddee7c0e5991958ec3463da0d (diff) | |
| download | samba-a4599b3011647c06be0d5a914bc4f72d490f6bc5.tar.gz | |
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
Diffstat (limited to 'source4/smbd')
| -rw-r--r-- | source4/smbd/service_stream.c | 5 |
1 files changed, 3 insertions, 2 deletions
diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c index 29e6515622b..bda28ad26f8 100644 --- 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); } /** |