Hi Duncan,
On Tue, May 07, 2024 at 09:17:19AM +1000, Duncan Roe wrote:
> 0c5e5fb introduced struct nfqnl_q_handle *qh_list which can point to
> dynamically acquired memory. Without this patch, that memory is not freed.
Indeed.
Looking at the example available at utils, I can see this assumes
that:
nfq_destroy_queue(qh);
needs to be called.
qh->data can be also set to heap structure, in that case this would leak too.
It seems nfq_destroy_queue() needs to be called before nfq_close() by design.
Probably add:
assert(h->qh_list == NULL);
at the top of nfq_close() instead to give a chance to users of this to
fix their code in case they are leaking qh?
Thanks
> Fixes: 0c5e5fb15205 ("sync with all 'upstream' changes in libnfnetlink_log")
> Signed-off-by: Duncan Roe <duncan_roe@xxxxxxxxxxxxxxx>
> ---
> src/libnetfilter_queue.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
> index bf67a19..f152efb 100644
> --- a/src/libnetfilter_queue.c
> +++ b/src/libnetfilter_queue.c
> @@ -481,7 +481,13 @@ EXPORT_SYMBOL
> int nfq_close(struct nfq_handle *h)
> {
> int ret;
> + struct nfq_q_handle *qh;
>
> + while (h->qh_list) {
> + qh = h->qh_list;
> + h->qh_list = qh->next;
> + free(qh);
> + }
> ret = nfnl_close(h->nfnlh);
> if (ret == 0)
> free(h);
> --
> 2.35.8
>
>
