BUG/MINOR: ssl: revert two wrong fixes with ckhi_link

This reverts commit 056ad01d55675ab2d65c7b41a2e1096db27b3d14.
This reverts commit ddd480cbdc0d54b3426ce9b6dd68cd849747cb07.

The architecture is ambiguous here: ckch_inst_free() is detaching and
freeing the "ckch_inst_link" linked list which must be free'd only from
the cafile_entry side.

The problem was also hidden by the fix ddd480c ("BUG/MEDIUM: ssl: Fix a
UAF when old ckch instances are released") which change the ckchi_link
inner loop by a safe one. However this can't fix entirely the problem
since both __ckch_inst_free_locked() could remove several nodes in the
ckchi_link linked list.

This revert is voluntary reintroducing a memory leak before really fixing
the problem.

Must be backported in 2.5 + 2.6.

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 6c8975f06ef7e01f32eabf458f910b5e54cddbdc..6db62e193c115e380b9b6e00884092d6ddf7d125 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -989,7 +989,6 @@ void ckch_inst_free(struct ckch_inst *inst)
        list_for_each_entry_safe(link_ref, link_ref_s, &inst->cafile_link_refs, list) {
                LIST_DELETE(&link_ref->link->list);
                LIST_DELETE(&link_ref->list);
-               free(link_ref->link);
                free(link_ref);
        }
 
@@ -2810,7 +2809,7 @@ static int cli_io_handler_commit_cafile_crlfile(struct appctx *appctx)
        int y = 0;
        struct cafile_entry *old_cafile_entry = ctx->old_entry;
        struct cafile_entry *new_cafile_entry = ctx->new_entry;
-       struct ckch_inst_link *ckchi_link, *ckchi_link_back;
+       struct ckch_inst_link *ckchi_link;
        char *path;
 
        if (unlikely(sc_ic(sc)->flags & (CF_WRITE_ERROR|CF_SHUTW)))
@@ -2910,7 +2909,7 @@ static int cli_io_handler_commit_cafile_crlfile(struct appctx *appctx)
                                }
 
                                /* delete the old sni_ctx, the old ckch_insts and the ckch_store */
-                               list_for_each_entry_safe(ckchi_link, ckchi_link_back, &old_cafile_entry->ckch_inst_link, list) {
+                               list_for_each_entry(ckchi_link, &old_cafile_entry->ckch_inst_link, list) {
                                        __ckch_inst_free_locked(ckchi_link->ckch_inst);
                                }