Broken allocation failure branches

[riastradh]

Description

npf_conn_establish (invoked from the packet-processing path in softint context) has an error branch to handle memory allocation failure in thmap_put (via npf_conndb_insert), but the error branch calls thmap_del (via npf_conndb_remove), which relies on memory allocation to succeed (rmind/thmap#11):

npf/src/kern/npf_conn.c

Lines 477 to 480 in 2efbe28

	if (!npf_conndb_insert(conn_db, bk, con, NPF_FLOW_BACK)) {
	npf_conn_t *ret __diagused;
	ret = npf_conndb_remove(conn_db, fw);
	KASSERT(ret == con);

This error branch is essentially guaranteed to crash -- see, e.g.: https://gnats.netbsd.org/57208

Environment and configuration

Environment:

• NPF environment: NetBSD
• Operating system version: NetBSD xxx.xxxx.net 9.2 NetBSD 9.2 (GENERIC) #0: Wed May 12 13:15:55 UTC 2021 mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64
• NPF version: NetBSD 9.2 (but the problem persists in npf master, NetBSD HEAD)

Configuration:
N/A

Any additional information

• For userspace where allocation is never guaranteed to succeed, thmap_del callers need to be taught to handle failure and retry later.
• For kernel where allocation can sleep, thmap_del needs to be made to sleep, and callers must defer it to thread context (with no spin locks held) where it can safely do so. npf_tableset.c needs to do this outside any spin locks.

[pettai]

Here's the configuration of the crashing system, FWIW

$wired_if = "bnx0"
$wired_v4 = inet4(bnx0)
$wired_v6 = inet6(bnx0)
$lab_if = inet4(bnx1)

table <blacklist> type hash file "/etc/npf_blacklist"
table <internal> type tree file "/etc/npf_internal"

$services_tcp = { domain, ssh, http, https }
$services_udp = { domain, ntp }

$internal_tcp = { 5666 }
$internal_udp = { bootps, syslog }

alg "icmp"

procedure "log" {
        # Note: npf_ext_log kernel module should be loaded, if not built-in.
        # Also, the interface created, e.g.: ifconfig npflog0 create
        log: npflog0
}

group "wired" on $wired_if {
        block in final from <blacklist>
        pass stateful in final family inet4 proto icmp to $wired_v4
        pass in final family inet6 proto ipv6-icmp to $wired_v6
        pass stateful in final family inet4 proto tcp to $wired_v4 port ssh apply "log"
        pass in final family inet6 proto tcp to $wired_v6 port ssh apply "log"
        pass in final family inet4 proto tcp from X.X.Y.Z to $wired_v4 port 782    # ConServer
        pass in final family inet4 proto tcp from X.X.Y.Z to $wired_v4 port 60000-65535    # ConServer
        pass stateful in final family inet4 proto tcp from <internal> to $wired_v4 port $internal_tcp
        pass stateful in final family inet4 proto udp from <internal> to $wired_v4 port $internal_tcp
        pass stateful in final family inet4 proto tcp to $wired_v4 port $services_tcp
        pass in final family inet6 proto tcp to $wired_v6 port $services_tcp
        pass stateful in final family inet4 proto udp to $wired_v4 port $services_udp
        pass in final family inet6 proto udp to $wired_v6 port $services_udp
        pass stateful in final family inet4 proto tcp to $wired_v4 port 49151-65535    # Passive FTP
        pass stateful in final family inet6 proto tcp to $wired_v6 port 49151-65535    # Passive FTP
        # pass stateful in final family inet4 proto udp to $wired_v4 port 33434-33600    # Traceroute
        # pass in final family inet6 proto udp to $wired_v6 port 33434-33600    # Traceroute

        # only SYN packets need to generate state
        pass stateful out final family inet6 proto tcp flags S/SA from $wired_v6
        pass stateful out final family inet4 proto tcp flags S/SA from $wired_v4
        # pass the other tcp packets without generating extra state
        pass out final family inet6 proto tcp from $wired_v6
        pass out final family inet4 proto tcp from $wired_v4

        # all other types of traffic, generate state per packet
        pass stateful out final family inet6 from $wired_v6
        pass stateful out final family inet4 from $wired_v4
}

group "lab" {
        pass final on $lab_if all
}

group default {
        pass final on lo0 all
        block all apply "log"
}