From 3d458f1b9f1055f1c8192e6bcd8103a7b68e7259 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 10 Sep 2026 14:09:01 +0200 Subject: [PATCH] iptables: handle missing rules on delete `DeleteJumpRule` and `Rule.Delete` first check whether a rule exists before deleting it. Another process may remove the rule between those operations, causing the subsequent delete to fail. With the iptables nft backend, deleting a rule that no longer exists returns ENOENT, reported as: ``` iptables: Bad rule (does a matching rule exist in that chain?). ``` This existing race became reproducible with the Debian 13 upgrade, where firewalld may remove Docker's rules during a reload before Docker recreates them. Docker then treats the failed delete as fatal even though both delete helpers define deletion of a non-existent rule as a no-op. Recognize the missing-rule diagnostic and preserve that contract when the rule disappears after the initial existence check. Add a test for the diagnostic returned when deleting a non-existent rule. The test exercises the delete error directly because reproducing the race itself deterministically would be timing-dependent. Signed-off-by: Sebastiaan van Stijn --- daemon/libnetwork/iptables/iptables.go | 30 +++++++++++++++++++-- daemon/libnetwork/iptables/iptables_test.go | 30 +++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/daemon/libnetwork/iptables/iptables.go b/daemon/libnetwork/iptables/iptables.go index 3d11cf07df..581327dc99 100644 --- a/daemon/libnetwork/iptables/iptables.go +++ b/daemon/libnetwork/iptables/iptables.go @@ -440,7 +440,11 @@ func (iptable IPTable) DeleteJumpRule(table Table, fromChain, toChain string, ru rule = append(rule, "-j", toChain) if iptable.Exists(table, fromChain, rule...) { if err := iptable.RawCombinedOutput(append([]string{"-t", string(table), "-D", fromChain}, rule...)...); err != nil { - return fmt.Errorf("unable to remove jump to %s rule in %s chain: %v", toChain, fromChain, err) + // Another process may have removed the rule between Exists and Delete. + if isRuleNotFoundError(err) { + return nil + } + return fmt.Errorf("unable to remove jump to %s rule in %s chain: %w", toChain, fromChain, err) } } return nil @@ -500,7 +504,14 @@ func (r Rule) Delete() error { if !r.Exists() { return nil } - return r.exec(Delete) + if err := r.exec(Delete); err != nil { + // Another process may have removed the rule between Exists and Delete. + if isRuleNotFoundError(err) { + return nil + } + return err + } + return nil } func (r Rule) String() string { @@ -510,3 +521,18 @@ func (r Rule) String() string { } return strings.Join(cmd, " ") } + +// isRuleNotFoundError reports whether err indicates that an iptables +// delete could not find a matching rule. +// +// The iptables-nft backend maps ENOENT from nft_rule_delete to this +// diagnostic: +// +// iptables: Bad rule (does a matching rule exist in that chain?). +// +// see: https://git.netfilter.org/iptables/tree/iptables/nft.c?id=384958620abab397062b67fb2763e813b63f74f0#n2745 +// ref: https://git.netfilter.org/iptables/commit/?id=384958620abab397062b67fb2763e813b63f74f0 +func isRuleNotFoundError(err error) bool { + return err != nil && + strings.Contains(err.Error(), "Bad rule (does a matching rule exist in that chain?)") +} diff --git a/daemon/libnetwork/iptables/iptables_test.go b/daemon/libnetwork/iptables/iptables_test.go index da74efb440..ed3f14b0fd 100644 --- a/daemon/libnetwork/iptables/iptables_test.go +++ b/daemon/libnetwork/iptables/iptables_test.go @@ -336,3 +336,33 @@ func TestFlushChain(t *testing.T) { // Cleanup _ = iptable.RemoveExistingChain(chain, table) } + +// TestDeleteNonExistingRule verifies the diagnostic returned by iptables when +// deleting a rule that does not exist. This is used to handle the race where a +// rule is removed by another process between checking for it and deleting it. +// +// Reproducing that race directly would be timing-dependent, so exercise the +// delete error deterministically with a rule in a freshly-created chain. +func TestDeleteNonExistingRule(t *testing.T) { + if UsingFirewalld() { + t.Skip("firewalld in host netns cannot create rules in the test's netns") + } + defer netnsutils.SetupTestOSContext(t)() + iptable := GetIptable(IPv4) + + const chain = "TEST-DELETEMISSING" + _, err := iptable.NewChain(chain, Filter) + assert.NilError(t, err) + defer func() { _ = iptable.RemoveExistingChain(chain, Filter) }() + + // Use TEST-NET-1 to make the rule clearly test-only. The freshly-created + // chain guarantees that the rule does not already exist. + err = iptable.RawCombinedOutput( + "-t", string(Filter), + "-D", chain, + "-s", "192.0.2.1", + "-j", "ACCEPT", + ) + assert.Assert(t, err != nil) + assert.Assert(t, isRuleNotFoundError(err), err) +}