Add cycle detection and jump recursion depth limit for netfilter.

In iptables, a malformed ruleset containing jump loops (e.g. a rule jumping
to itself or a cycle of jumps between user-defined chains) previously caused
checkChain to recurse infinitely during packet evaluation, triggering a
goroutine stack overflow (runtime: goroutine stack exceeds 1000000000-byte
limit).

assisted-by:agy
PiperOrigin-RevId: 959981821

pkg/sentry/socket/netfilter/BUILD

@@ -60,11 +60,15 @@ go_test(
 
 go_test(
     name = "netfilter_test",
-    srcs = ["mark_matcher_test.go"],
+    srcs = [
+        "mark_matcher_test.go",
+        "netfilter_test.go",
+    ],
     library = ":netfilter",
     deps = [
         "//pkg/abi/linux",
         "//pkg/marshal",
+        "//pkg/syserr",
         "//pkg/tcpip/stack",
     ],
 )

pkg/sentry/socket/netfilter/netfilter.go

@@ -294,10 +294,9 @@ func SetEntries(mapper IDMapper, stk *stack.Stack, optVal []byte, ipv6 bool) *sy
 		table.Rules[ruleIdx] = rule
 	}
 
-	// TODO(gvisor.dev/issue/6167): Check the following conditions:
-	//	- There are no loops.
-	//	- There are no chains without an unconditional final rule.
-	//	- There are no chains without an unconditional underflow rule.
+	if err := checkLoopsAndChains(table, ipv6); err != nil {
+		return err
+	}
 
 	stk.IPTables().ReplaceTable(nameToID[replace.Name.String()], table, ipv6)
 	return nil
@@ -432,6 +431,99 @@ func isUnconditionalAccept(rule stack.Rule, ipv6 bool) bool {
 	return ok
 }
 
+func isUnconditionalFinalRule(rule stack.Rule, ipv6 bool) bool {
+	if len(rule.Matchers) != 0 {
+		return false
+	}
+	if (ipv6 && rule.Filter != emptyIPv6Filter) || (!ipv6 && rule.Filter != emptyIPv4Filter) {
+		return false
+	}
+	if _, ok := rule.Target.(*JumpTarget); ok {
+		return false
+	}
+	if isUserChainTarget(rule.Target) {
+		return false
+	}
+	return true
+}
+
+type visitState uint8
+
+const (
+	unvisited visitState = iota
+	visiting
+	visited
+)
+
+func checkChainDFS(table stack.Table, ruleIdx int, state []visitState, ipv6 bool) *syserr.Error {
+	if state[ruleIdx] == visiting {
+		nflog("jump loop detected at rule %d", ruleIdx)
+		return syserr.ErrInvalidArgument
+	}
+	if state[ruleIdx] == visited {
+		return nil
+	}
+	state[ruleIdx] = visiting
+
+	rule := table.Rules[ruleIdx]
+	if jump, ok := rule.Target.(*JumpTarget); ok {
+		jumpTo := jump.RuleNum
+		if jumpTo <= 0 || jumpTo >= len(table.Rules) {
+			nflog("jump to out-of-bounds rule %d", jumpTo)
+			return syserr.ErrInvalidArgument
+		}
+		if !isUserChainTarget(table.Rules[jumpTo-1].Target) {
+			nflog("jump target %d is not a user chain (rule %d-1 target is %T)", jumpTo, jumpTo, table.Rules[jumpTo-1].Target)
+			return syserr.ErrInvalidArgument
+		}
+		if err := checkChainDFS(table, jumpTo, state, ipv6); err != nil {
+			return err
+		}
+	}
+
+	if isUnconditionalFinalRule(rule, ipv6) {
+		state[ruleIdx] = visited
+		return nil
+	}
+
+	nextIdx := ruleIdx + 1
+	if nextIdx >= len(table.Rules) {
+		nflog("chain fell off the end of table without an unconditional final rule at rule %d", ruleIdx)
+		return syserr.ErrInvalidArgument
+	}
+	if isUserChainTarget(table.Rules[nextIdx].Target) {
+		nflog("chain fell through into a user chain without an unconditional final rule at rule %d (next target is %T)", ruleIdx, table.Rules[nextIdx].Target)
+		return syserr.ErrInvalidArgument
+	}
+	if err := checkChainDFS(table, nextIdx, state, ipv6); err != nil {
+		return err
+	}
+	state[ruleIdx] = visited
+	return nil
+}
+
+func checkLoopsAndChains(table stack.Table, ipv6 bool) *syserr.Error {
+	state := make([]visitState, len(table.Rules))
+
+	for hook, ruleIdx := range table.BuiltinChains {
+		if table.ValidHooks()&(1<<hook) != 0 {
+			if err := checkChainDFS(table, ruleIdx, state, ipv6); err != nil {
+				return err
+			}
+		}
+	}
+
+	for ruleIdx, rule := range table.Rules {
+		if isUserChainTarget(rule.Target) && ruleIdx+1 < len(table.Rules) {
+			if err := checkChainDFS(table, ruleIdx+1, state, ipv6); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 func hookFromLinux(hook int) stack.Hook {
 	switch hook {
 	case linux.NF_INET_PRE_ROUTING:

pkg/sentry/socket/netfilter/netfilter_test.go

@@ -0,0 +1,141 @@
+// Copyright 2026 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package netfilter
+
+import (
+	"testing"
+
+	"gvisor.dev/gvisor/pkg/syserr"
+	"gvisor.dev/gvisor/pkg/tcpip/stack"
+)
+
+func makeRule(target stack.Target, matchers ...stack.Matcher) stack.Rule {
+	return stack.Rule{
+		Filter:   emptyIPv4Filter,
+		Target:   target,
+		Matchers: matchers,
+	}
+}
+
+func makeBuiltinChains(prerouting int) [stack.NumHooks]int {
+	return [stack.NumHooks]int{
+		stack.Prerouting:  prerouting,
+		stack.Input:       stack.HookUnset,
+		stack.Forward:     stack.HookUnset,
+		stack.Output:      stack.HookUnset,
+		stack.Postrouting: stack.HookUnset,
+	}
+}
+
+func makeUnderflows(prerouting int) [stack.NumHooks]int {
+	return [stack.NumHooks]int{
+		stack.Prerouting:  prerouting,
+		stack.Input:       stack.HookUnset,
+		stack.Forward:     stack.HookUnset,
+		stack.Output:      stack.HookUnset,
+		stack.Postrouting: stack.HookUnset,
+	}
+}
+
+func TestCheckLoopsAndChainsDirectLoop(t *testing.T) {
+	// Rule 0 (builtin hook Prerouting) jumps to itself (Rule 0). This mirrors
+	// the syzkaller reproducer where a rule's target offset jumps to itself.
+	table := stack.Table{
+		Rules: []stack.Rule{
+			makeRule(&JumpTarget{RuleNum: 0}),
+		},
+		BuiltinChains: makeBuiltinChains(0),
+		Underflows:    makeUnderflows(0),
+	}
+	if err := checkLoopsAndChains(table, false); err != syserr.ErrInvalidArgument {
+		t.Fatalf("checkLoopsAndChains expected %v for direct jump loop, got %v", syserr.ErrInvalidArgument, err)
+	}
+}
+
+func TestCheckLoopsAndChainsIndirectLoop(t *testing.T) {
+	// Builtin chain Prerouting (Rule 0) jumps to user chain A (Rule 3).
+	// User chain A (Rule 3) jumps to user chain B (Rule 6).
+	// User chain B (Rule 6) jumps back to user chain A (Rule 3), forming a cycle.
+	table := stack.Table{
+		Rules: []stack.Rule{
+			makeRule(&JumpTarget{RuleNum: 3}), // Rule 0: Prerouting -> jump to Rule 3
+			makeRule(&acceptTarget{}),         // Rule 1: Prerouting underflow
+			makeRule(&userChainTarget{}),      // Rule 2: user chain A header
+			makeRule(&JumpTarget{RuleNum: 6}), // Rule 3: user chain A rule -> jump to Rule 6
+			makeRule(&returnTarget{}),         // Rule 4: user chain A return
+			makeRule(&userChainTarget{}),      // Rule 5: user chain B header
+			makeRule(&JumpTarget{RuleNum: 3}), // Rule 6: user chain B rule -> jump back to Rule 3 (cycle!)
+			makeRule(&returnTarget{}),         // Rule 7: user chain B return
+		},
+		BuiltinChains: makeBuiltinChains(0),
+		Underflows:    makeUnderflows(1),
+	}
+	if err := checkLoopsAndChains(table, false); err != syserr.ErrInvalidArgument {
+		t.Fatalf("checkLoopsAndChains expected %v for indirect jump loop, got %v", syserr.ErrInvalidArgument, err)
+	}
+}
+
+func TestCheckLoopsAndChainsJumpToNonUserChain(t *testing.T) {
+	// Builtin chain Prerouting (Rule 0) jumps to Rule 2, which is not
+	// preceded by a userChainTarget.
+	table := stack.Table{
+		Rules: []stack.Rule{
+			makeRule(&JumpTarget{RuleNum: 2}), // Rule 0: Prerouting -> jump to Rule 2
+			makeRule(&acceptTarget{}),         // Rule 1: Prerouting underflow
+			makeRule(&acceptTarget{}),         // Rule 2: not a user chain
+		},
+		BuiltinChains: makeBuiltinChains(0),
+		Underflows:    makeUnderflows(1),
+	}
+	if err := checkLoopsAndChains(table, false); err != syserr.ErrInvalidArgument {
+		t.Fatalf("checkLoopsAndChains expected %v for jump to non-user chain, got %v", syserr.ErrInvalidArgument, err)
+	}
+}
+
+func TestCheckLoopsAndChainsNoUnconditionalFinalRule(t *testing.T) {
+	// User chain A (Rule 3) ends without an unconditional final rule before
+	// falling off the end of table.Rules.
+	table := stack.Table{
+		Rules: []stack.Rule{
+			makeRule(&JumpTarget{RuleNum: 3}),         // Rule 0: Prerouting -> jump to Rule 3
+			makeRule(&acceptTarget{}),                 // Rule 1: Prerouting underflow
+			makeRule(&userChainTarget{}),              // Rule 2: user chain A header
+			makeRule(&returnTarget{}, &MarkMatcher{}), // Rule 3: conditional return
+		},
+		BuiltinChains: makeBuiltinChains(0),
+		Underflows:    makeUnderflows(1),
+	}
+	if err := checkLoopsAndChains(table, false); err != syserr.ErrInvalidArgument {
+		t.Fatalf("checkLoopsAndChains expected %v for unterminated user chain, got %v", syserr.ErrInvalidArgument, err)
+	}
+}
+
+func TestCheckLoopsAndChainsValid(t *testing.T) {
+	// Valid ruleset: Prerouting (Rule 0) jumps to user chain A (Rule 3).
+	// User chain A (Rule 3) returns unconditionally.
+	table := stack.Table{
+		Rules: []stack.Rule{
+			makeRule(&JumpTarget{RuleNum: 3}), // Rule 0: Prerouting -> jump to Rule 3
+			makeRule(&acceptTarget{}),         // Rule 1: Prerouting underflow
+			makeRule(&userChainTarget{}),      // Rule 2: user chain A header
+			makeRule(&returnTarget{}),         // Rule 3: unconditional RETURN
+		},
+		BuiltinChains: makeBuiltinChains(0),
+		Underflows:    makeUnderflows(1),
+	}
+	if err := checkLoopsAndChains(table, false); err != nil {
+		t.Fatalf("checkLoopsAndChains expected nil for valid table, got %v", err)
+	}
+}

pkg/tcpip/stack/iptables.go

@@ -21,6 +21,7 @@ import (
 	"reflect"
 	"time"
 
+	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/tcpip"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
 )
@@ -647,7 +648,7 @@ func check(it *IPTables, table Table, hook Hook, pkt *PacketBuffer, r *Route, ad
 // Precondition: The packet's network and transport header must be set.
 func (it *IPTables) check(table Table, hook Hook, pkt *PacketBuffer, r *Route, addressEP AddressableEndpoint, inNicName, outNicName string) bool {
 	ruleIdx := table.BuiltinChains[hook]
-	switch verdict := it.checkChain(hook, pkt, table, ruleIdx, r, addressEP, inNicName, outNicName); verdict {
+	switch verdict := it.checkChain(hook, pkt, table, ruleIdx, r, addressEP, inNicName, outNicName, 0 /* depth */); verdict {
 	// If the table returns Accept, move on to the next table.
 	case chainAccept:
 		return true
@@ -695,10 +696,19 @@ func (it *IPTables) startReaper(interval time.Duration) {
 	})
 }
 
+// maxJumps is the maximum allowed depth of recursion when jumping between
+// chains in iptables. This prevents stack overflows from jump loops or deep
+// recursion. Linux kernel uses XT_JUMP_STACK_MAX = 16.
+const maxJumps = 16
+
 // Preconditions:
 //   - pkt is a IPv4 packet of at least length header.IPv4MinimumSize.
 //   - pkt.NetworkHeader is not nil.
-func (it *IPTables) checkChain(hook Hook, pkt *PacketBuffer, table Table, ruleIdx int, r *Route, addressEP AddressableEndpoint, inNicName, outNicName string) chainVerdict {
+func (it *IPTables) checkChain(hook Hook, pkt *PacketBuffer, table Table, ruleIdx int, r *Route, addressEP AddressableEndpoint, inNicName, outNicName string, depth int) chainVerdict {
+	if depth >= maxJumps {
+		log.Warningf("iptables: jump depth exceeded max %d", maxJumps)
+		return chainDrop
+	}
 	// Start from ruleIdx and walk the list of rules until a rule gives us
 	// a verdict.
 	for ruleIdx < len(table.Rules) {
@@ -719,7 +729,7 @@ func (it *IPTables) checkChain(hook Hook, pkt *PacketBuffer, table Table, ruleId
 				ruleIdx++
 				continue
 			}
-			switch verdict := it.checkChain(hook, pkt, table, jumpTo, r, addressEP, inNicName, outNicName); verdict {
+			switch verdict := it.checkChain(hook, pkt, table, jumpTo, r, addressEP, inNicName, outNicName, depth+1); verdict {
 			case chainAccept:
 				return chainAccept
 			case chainDrop:

pkg/tcpip/stack/iptables_test.go

@@ -429,3 +429,47 @@ func TestNATConflict(t *testing.T) {
 		})
 	}
 }
+
+type jumpTarget struct {
+	target int
+}
+
+func (jt *jumpTarget) Action(*PacketBuffer, Hook, *Route, AddressableEndpoint) (RuleVerdict, int) {
+	return RuleJump, jt.target
+}
+
+func TestIPTablesJumpLoop(t *testing.T) {
+	iptables := DefaultTables(faketime.NewManualClock(), rand.New(rand.NewSource(0)))
+	table := Table{
+		Rules: []Rule{
+			// Rule 0 jumps to itself, forming an infinite loop.
+			{
+				Target: &jumpTarget{target: 0},
+			},
+			{
+				Target: &AcceptTarget{},
+			},
+		},
+		BuiltinChains: [NumHooks]int{
+			Prerouting:  0,
+			Input:       0,
+			Forward:     0,
+			Output:      0,
+			Postrouting: 0,
+		},
+		Underflows: [NumHooks]int{
+			Prerouting:  1,
+			Input:       1,
+			Forward:     1,
+			Output:      1,
+			Postrouting: 1,
+		},
+	}
+	iptables.ReplaceTable(FilterID, table, true /* ipv6 */)
+
+	// This should drop the packet (due to exceeding maxJumps) without a stack overflow.
+	pkt := v6PacketBuffer()
+	if iptables.CheckInput(pkt, "") {
+		t.Errorf("CheckInput with jump loop expected to drop packet, but got accept")
+	}
+}