From 0bfa3e7ff28942b374a937f5d307b1007b076545 Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Tue, 30 Aug 2022 17:32:38 +0200 Subject: [PATCH] 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. --- src/ssl_ckch.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index 6c8975f06e..6db62e193c 100644 --- 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); }