[BUG] fix random memory corruption using "show sess"

Commit 8a5c626e73bac905d150185e45110525588d7b4c introduced the sessions
dump on the unix socket. This implementation is buggy because it may try
to link to the sessions list's head after the last session is removed
with a backref. Also, for the LIST_ISEMPTY test to succeed, we have to
proceed with LIST_INIT after LIST_DEL.

diff --git a/src/client.c b/src/client.c
index bf8519f177203f2a893c2c08d493d45dd1dbb263..4cc3d7ea03038957fe1a3dfbc8838f5b4244e334 100644 (file)
--- a/src/client.c
+++ b/src/client.c
@@ -66,7 +66,6 @@ int event_accept(int fd) {
        struct session *s;
        struct http_txn *txn;
        struct task *t;
-       struct bref *bref, *back;
        int cfd;
        int max_accept = global.tune.maxaccept;
 
@@ -470,11 +469,6 @@ int event_accept(int fd) {
  out_free_task:
        pool_free2(pool2_task, t);
  out_free_session:
-       list_for_each_entry_safe(bref, back, &s->back_refs, users) {
-               LIST_DEL(&bref->users);
-               LIST_ADDQ(&LIST_ELEM(s->list.n, struct session *, list)->back_refs, &bref->users);
-               bref->ref = s->list.n;
-       }
        LIST_DEL(&s->list);
        pool_free2(pool2_session, s);
  out_close:

diff --git a/src/dumpstats.c b/src/dumpstats.c
index 07e505649dea18a232d1a64cd10d9c47ca01bb0f..b12fbd0b1ef226afe26c0dbdd3b2facde2d68165 100644 (file)
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -1099,8 +1099,10 @@ void stats_dump_sess_to_buffer(struct session *s, struct buffer *rep)
                 * reference to the last session being dumped.
                 */
                if (s->data_state == DATA_ST_LIST) {
-                       if (!LIST_ISEMPTY(&s->data_ctx.sess.bref.users))
+                       if (!LIST_ISEMPTY(&s->data_ctx.sess.bref.users)) {
                                LIST_DEL(&s->data_ctx.sess.bref.users);
+                               LIST_INIT(&s->data_ctx.sess.bref.users);
+                       }
                }
                s->data_state = DATA_ST_FIN;
                buffer_stop_hijack(rep);
@@ -1118,7 +1120,10 @@ void stats_dump_sess_to_buffer(struct session *s, struct buffer *rep)
        case DATA_ST_INIT:
                /* the function had not been called yet, let's prepare the
                 * buffer for a response. We initialize the current session
-                * pointer to the first in the global list.
+                * pointer to the first in the global list. When a target
+                * session is being destroyed, it is responsible for updating
+                * this pointer. We know we have reached the end when this
+                * pointer points back to the head of the sessions list.
                 */
                stream_int_retnclose(rep->cons, &msg);
                LIST_INIT(&s->data_ctx.sess.bref.users);
@@ -1127,16 +1132,19 @@ void stats_dump_sess_to_buffer(struct session *s, struct buffer *rep)
                /* fall through */
 
        case DATA_ST_LIST:
+               /* first, let's detach the back-ref from a possible previous session */
+               if (!LIST_ISEMPTY(&s->data_ctx.sess.bref.users)) {
+                       LIST_DEL(&s->data_ctx.sess.bref.users);
+                       LIST_INIT(&s->data_ctx.sess.bref.users);
+               }
+
+               /* and start from where we stopped */
                while (s->data_ctx.sess.bref.ref != &sessions) {
                        char pn[INET6_ADDRSTRLEN + strlen(":65535")];
                        struct session *curr_sess;
 
                        curr_sess = LIST_ELEM(s->data_ctx.sess.bref.ref, struct session *, list);
 
-                       /* first, let's detach the back-ref from a possible previous session */
-                       if (!LIST_ISEMPTY(&s->data_ctx.sess.bref.users))
-                               LIST_DEL(&s->data_ctx.sess.bref.users);
-
                        chunk_printf(&msg, sizeof(trash),
                                     "%p: proto=%s",
                                     curr_sess,
@@ -1194,7 +1202,9 @@ void stats_dump_sess_to_buffer(struct session *s, struct buffer *rep)
                                             : "never");
 
                        if (buffer_write_chunk(rep, &msg) >= 0) {
-                               /* let's try again later */
+                               /* let's try again later from this session. We add ourselves into
+                                * this session's users so that it can remove us upon termination.
+                                */
                                LIST_ADDQ(&curr_sess->back_refs, &s->data_ctx.sess.bref.users);
                                return;
                        }

diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index f07fa158bb03f85a42dc3376659c8bca2a291615..997cf496eb5055908c411f04529833cf27a79bde 100644 (file)
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -360,7 +360,6 @@ int uxst_event_accept(int fd) {
        struct listener *l = fdtab[fd].owner;
        struct session *s;
        struct task *t;
-       struct bref *bref, *back;
        int cfd;
        int max_accept;
 
@@ -553,11 +552,6 @@ int uxst_event_accept(int fd) {
  out_free_task:
        pool_free2(pool2_task, t);
  out_free_session:
-       list_for_each_entry_safe(bref, back, &s->back_refs, users) {
-               LIST_DEL(&bref->users);
-               LIST_ADDQ(&LIST_ELEM(s->list.n, struct session *, list)->back_refs, &bref->users);
-               bref->ref = s->list.n;
-       }
        LIST_DEL(&s->list);
        pool_free2(pool2_session, s);
  out_close:

diff --git a/src/session.c b/src/session.c
index d1bc052aa382a55542c85fab62f88829f1945659..8ced9b8bf30b8987a3fbe62ec71b7d9fc14a0afe 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -98,8 +98,13 @@ void session_free(struct session *s)
        pool_free2(pool2_capture, txn->srv_cookie);
 
        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.
+                */
                LIST_DEL(&bref->users);
-               LIST_ADDQ(&LIST_ELEM(s->list.n, struct session *, list)->back_refs, &bref->users);
+               LIST_INIT(&bref->users);
+               if (s->list.n != &sessions)
+                       LIST_ADDQ(&LIST_ELEM(s->list.n, struct session *, list)->back_refs, &bref->users);
                bref->ref = s->list.n;
        }
        LIST_DEL(&s->list);