conntrack: prevent potential memory leak

Currently, the ConntrackDeleteFilters captures all flow entries
it fails to delete and reports them as errors. This behavior
can potentially lead to memory leaks in high-traffic systems,
where thousands of conntrack flow entries are cleared in a single
batch. With this commit, instead of returning all the un-deleted
flow entries, we now return a single error message for all of them.

Signed-off-by: Daman Arora <aroradaman@gmail.com>
This commit is contained in:
Daman Arora
2025-02-06 16:10:06 +05:30
committed by Alessandro Boch
parent 7c2350bd14
commit 62fb240731

View File

@@ -5,8 +5,8 @@ import (
"encoding/binary" "encoding/binary"
"errors" "errors"
"fmt" "fmt"
"io/fs"
"net" "net"
"strings"
"time" "time"
"github.com/vishvananda/netlink/nl" "github.com/vishvananda/netlink/nl"
@@ -159,7 +159,7 @@ func (h *Handle) ConntrackDeleteFilter(table ConntrackTableType, family InetFami
// ConntrackDeleteFilters deletes entries on the specified table matching any of the specified filters using the netlink handle passed // ConntrackDeleteFilters deletes entries on the specified table matching any of the specified filters using the netlink handle passed
// conntrack -D [table] parameters Delete conntrack or expectation // conntrack -D [table] parameters Delete conntrack or expectation
func (h *Handle) ConntrackDeleteFilters(table ConntrackTableType, family InetFamily, filters ...CustomConntrackFilter) (uint, error) { func (h *Handle) ConntrackDeleteFilters(table ConntrackTableType, family InetFamily, filters ...CustomConntrackFilter) (uint, error) {
var errMsgs []string var finalErr error
res, err := h.dumpConntrackTable(table, family) res, err := h.dumpConntrackTable(table, family)
if err != nil { if err != nil {
if !errors.Is(err, ErrDumpInterrupted) { if !errors.Is(err, ErrDumpInterrupted) {
@@ -167,9 +167,10 @@ func (h *Handle) ConntrackDeleteFilters(table ConntrackTableType, family InetFam
} }
// This allows us to at least do a best effort to try to clean the // This allows us to at least do a best effort to try to clean the
// entries matching the filter. // entries matching the filter.
errMsgs = append(errMsgs, err.Error()) finalErr = err
} }
var totalFilterErrors int
var matched uint var matched uint
for _, dataRaw := range res { for _, dataRaw := range res {
flow := parseRawData(dataRaw) flow := parseRawData(dataRaw)
@@ -178,19 +179,20 @@ func (h *Handle) ConntrackDeleteFilters(table ConntrackTableType, family InetFam
req2 := h.newConntrackRequest(table, family, nl.IPCTNL_MSG_CT_DELETE, unix.NLM_F_ACK) req2 := h.newConntrackRequest(table, family, nl.IPCTNL_MSG_CT_DELETE, unix.NLM_F_ACK)
// skip the first 4 byte that are the netfilter header, the newConntrackRequest is adding it already // skip the first 4 byte that are the netfilter header, the newConntrackRequest is adding it already
req2.AddRawData(dataRaw[4:]) req2.AddRawData(dataRaw[4:])
if _, err = req2.Execute(unix.NETLINK_NETFILTER, 0); err == nil { if _, err = req2.Execute(unix.NETLINK_NETFILTER, 0); err == nil || errors.Is(err, fs.ErrNotExist) {
matched++ matched++
// flow is already deleted, no need to match on other filters and continue to the next flow. // flow is already deleted, no need to match on other filters and continue to the next flow.
break break
} } else {
errMsgs = append(errMsgs, fmt.Sprintf("failed to delete conntrack flow '%s': %s", flow.String(), err.Error())) totalFilterErrors++
} }
} }
} }
if len(errMsgs) > 0 {
return matched, fmt.Errorf(strings.Join(errMsgs, "; "))
} }
return matched, nil if totalFilterErrors > 0 {
finalErr = errors.Join(finalErr, fmt.Errorf("failed to delete %d conntrack flows with %d filters", totalFilterErrors, len(filters)))
}
return matched, finalErr
} }
func (h *Handle) newConntrackRequest(table ConntrackTableType, family InetFamily, operation, flags int) *nl.NetlinkRequest { func (h *Handle) newConntrackRequest(table ConntrackTableType, family InetFamily, operation, flags int) *nl.NetlinkRequest {