From 8f63a8f3bffbbaf8fae4985ee5f974ab326b08c0 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 7 Apr 2023 16:02:41 +0200 Subject: [PATCH] http1: remove transactions from their list instead of keeping a NULL pointer in an array Ticket: #5921 --- src/app-layer-htp-file.c | 3 ++- src/app-layer-htp.c | 16 +++++++++------- src/app-layer-htp.h | 7 +++++++ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/app-layer-htp-file.c b/src/app-layer-htp-file.c index f96b370160..7b3ba62edc 100644 --- a/src/app-layer-htp-file.c +++ b/src/app-layer-htp-file.c @@ -179,7 +179,8 @@ int HTPFileOpenWithRange(HtpState *s, HtpTxUserData *txud, const uint8_t *filena } // Then, we will try to handle reassembly of different ranges of the same file - htp_tx_t *tx = htp_list_get(s->conn->transactions, txid); + // TODO have the caller pass directly the tx + htp_tx_t *tx = htp_list_get(s->conn->transactions, txid - s->tx_freed); if (!tx) { SCReturnInt(-1); } diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 1e8c0b8ea6..babb87d283 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -401,7 +401,7 @@ void HTPStateFree(void *state) uint64_t total_txs = HTPStateGetTxCnt(state); /* free the list of body chunks */ if (s->conn != NULL) { - for (tx_id = 0; tx_id < total_txs; tx_id++) { + for (tx_id = s->tx_freed; tx_id < total_txs; tx_id++) { htp_tx_t *tx = HTPStateGetTx(s, tx_id); if (tx != NULL) { HtpTxUserData *htud = (HtpTxUserData *) htp_tx_get_user_data(tx); @@ -458,8 +458,10 @@ static void HTPStateTransactionFree(void *state, uint64_t id) tx->request_progress = HTP_REQUEST_COMPLETE; tx->response_progress = HTP_RESPONSE_COMPLETE; } + // replaces tx in the s->conn->transactions list by NULL htp_tx_destroy(tx); } + s->tx_freed += htp_connp_tx_freed(s->connp); } /** @@ -3076,7 +3078,7 @@ static uint64_t HTPStateGetTxCnt(void *alstate) if (size < 0) return 0ULL; SCLogDebug("size %"PRIu64, size); - return (uint64_t)size; + return (uint64_t)size + http_state->tx_freed; } else { return 0ULL; } @@ -3086,8 +3088,8 @@ static void *HTPStateGetTx(void *alstate, uint64_t tx_id) { HtpState *http_state = (HtpState *)alstate; - if (http_state != NULL && http_state->conn != NULL) - return htp_list_get(http_state->conn->transactions, tx_id); + if (http_state != NULL && http_state->conn != NULL && tx_id >= http_state->tx_freed) + return htp_list_get(http_state->conn->transactions, tx_id - http_state->tx_freed); else return NULL; } @@ -3097,9 +3099,9 @@ void *HtpGetTxForH2(void *alstate) // gets last transaction HtpState *http_state = (HtpState *)alstate; if (http_state != NULL && http_state->conn != NULL) { - size_t txid = htp_list_array_size(http_state->conn->transactions); - if (txid > 0) { - return htp_list_get(http_state->conn->transactions, txid - 1); + size_t txid = HTPStateGetTxCnt(http_state); + if (txid > http_state->tx_freed) { + return htp_list_get(http_state->conn->transactions, txid - http_state->tx_freed - 1); } } return NULL; diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index dee5c17e83..5972bdaf50 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -247,6 +247,13 @@ typedef struct HtpState_ { htp_conn_t *conn; Flow *f; /**< Needed to retrieve the original flow when using HTPLib callbacks */ uint64_t transaction_cnt; + // tx_freed is the number of already freed transactions + // This is needed as libhtp only keeps the live transactions : + // To get the total number of transactions, we need to add + // the number of transactions tracked by libhtp to this number. + // It is also needed as an offset to translate between suricata + // transaction id to libhtp offset in its list/array + uint64_t tx_freed; const struct HTPCfgRec_ *cfg; uint16_t flags; uint16_t events;