From 4602363f6acff6466cf139a4ae7e8b5c3b5b9b39 Mon Sep 17 00:00:00 2001 From: Willy Tarreau <w@1wt.eu> Date: Thu, 7 Jan 2010 22:51:47 +0100 Subject: [PATCH] [BUG] http: fix for capture memory leak was incorrect That patch was incorrect because under some circumstances, the capture memory could be freed by session_free() and then again by http_end_txn(), causing a double free and an eventual segfault. The pool use count was also reported wrong due to this bug. The cleanup code was removed from session_free() to remain only in http_end_txn(). --- src/proto_http.c | 27 +++++++++++++++++---------- src/session.c | 19 ++++--------------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/proto_http.c b/src/proto_http.c index 9b9176cae..a3354207a 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -6310,19 +6310,11 @@ void http_init_txn(struct session *s) if (fe->options2 & PR_O2_REQBUG_OK) txn->req.err_pos = -1; /* let buggy requests pass */ - if (txn->req.cap) { - struct cap_hdr *h; - for (h = fe->req_cap; h; h = h->next) - pool_free2(h->pool, txn->req.cap[h->index]); + if (txn->req.cap) memset(txn->req.cap, 0, fe->nb_req_cap * sizeof(void *)); - } - if (txn->rsp.cap) { - struct cap_hdr *h; - for (h = fe->rsp_cap; h; h = h->next) - pool_free2(h->pool, txn->rsp.cap[h->index]); + if (txn->rsp.cap) memset(txn->rsp.cap, 0, fe->nb_rsp_cap * sizeof(void *)); - } if (txn->hdr_idx.v) hdr_idx_init(&txn->hdr_idx); @@ -6340,6 +6332,21 @@ void http_end_txn(struct session *s) txn->uri = NULL; txn->srv_cookie = NULL; txn->cli_cookie = NULL; + + if (txn->req.cap) { + struct cap_hdr *h; + for (h = s->fe->req_cap; h; h = h->next) + pool_free2(h->pool, txn->req.cap[h->index]); + memset(txn->req.cap, 0, s->fe->nb_req_cap * sizeof(void *)); + } + + if (txn->rsp.cap) { + struct cap_hdr *h; + for (h = s->fe->rsp_cap; h; h = h->next) + pool_free2(h->pool, txn->rsp.cap[h->index]); + memset(txn->rsp.cap, 0, s->fe->nb_rsp_cap * sizeof(void *)); + } + } /* to be used at the end of a transaction to prepare a new one */ diff --git a/src/session.c b/src/session.c index 406d2c0bd..48c8ce9c9 100644 --- a/src/session.c +++ b/src/session.c @@ -81,25 +81,14 @@ void session_free(struct session *s) if (s->sessid) pool_free2(apools.sessid, s->sessid); + http_end_txn(s); + if (fe) { pool_free2(fe->hdr_idx_pool, txn->hdr_idx.v); - - if (txn->rsp.cap != NULL) { - struct cap_hdr *h; - for (h = fe->rsp_cap; h; h = h->next) - pool_free2(h->pool, txn->rsp.cap[h->index]); - pool_free2(fe->rsp_cap_pool, txn->rsp.cap); - } - if (txn->req.cap != NULL) { - struct cap_hdr *h; - for (h = fe->req_cap; h; h = h->next) - pool_free2(h->pool, txn->req.cap[h->index]); - pool_free2(fe->req_cap_pool, txn->req.cap); - } + pool_free2(fe->rsp_cap_pool, txn->rsp.cap); + pool_free2(fe->req_cap_pool, txn->req.cap); } - http_end_txn(s); - list_for_each_entry_safe(bref, back, &s->back_refs, users) { /* we have to unlink all watchers. We must not relink them if * this session was the last one in the list.