From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f169.google.com (mail-pg1-f169.google.com [209.85.215.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E5CC337BE96 for ; Wed, 29 Jul 2026 06:08:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785305312; cv=none; b=E6uCCRAvpfMr3WPzbgdiXXKKdGkLSR8D0iuYgh0UqxnpO/xDGQMXSBkLRbCMSpFGQyg4yq74NBNqfchXJy2JK5KSn59RWJK14oZjnD843eceuv6bfJvW4rJjsHbTJKJh0t8qjS61T0dLFDhM+EkH31HAH6r4gC/KPzqSsUnf2qw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785305312; c=relaxed/simple; bh=Nt9jUCfpL9SHefZJ+JX8jYz95zpvBQciVw9B/Vp2MJ4=; h=Mime-Version:Content-Type:Date:Message-Id:From:To:Cc:Subject: References:In-Reply-To; b=A3yy65JoZpJlFBXT1DqLETNJ6aKxD5JP0D49F0lf/Hfo4KNchROZHsnFptwIv5fSXG7QS/a30aq/NM+gCZQp8l7GhzQPKs3mqtm7V2slyTVs0YVrKPkItrKt333E6fxIm2wjKPkAc4TTMpEnHrNioFbcNKmlvsWm/Xix5Fyw47U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=etsalapatis.com; spf=pass smtp.mailfrom=etsalapatis.com; dkim=pass (2048-bit key) header.d=etsalapatis-com.20251104.gappssmtp.com header.i=@etsalapatis-com.20251104.gappssmtp.com header.b=PD6Dwls1; arc=none smtp.client-ip=209.85.215.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=etsalapatis.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=etsalapatis.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=etsalapatis-com.20251104.gappssmtp.com header.i=@etsalapatis-com.20251104.gappssmtp.com header.b="PD6Dwls1" Received: by mail-pg1-f169.google.com with SMTP id 41be03b00d2f7-c9e7391839cso529499a12.0 for ; Tue, 28 Jul 2026 23:08:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=etsalapatis-com.20251104.gappssmtp.com; s=20251104; t=1785305309; x=1785910109; darn=vger.kernel.org; h=in-reply-to:references:subject:cc:to:from:message-id:date :content-type:content-transfer-encoding:mime-version:from:to:cc :subject:date:message-id:reply-to:content-type; bh=sK/uHPOHKOSpL2rOWH/430U5Wo4SeZ5sprv1VG2eCss=; b=PD6Dwls1wCezmN6sm8uSN53WJAUFKZ6ifQTrDGx75mXl6qwoXyjH7X3XcX5FQXHxPM sONoVCRp3yGAcj7djnuqOUr3KsJEsKypxqy6LrsEBynHHMRp7yCXSQgZ2FIGvJdtpVbs zK+XfC907jjB7NYInQi8IU/ldmGPiE5BPqwcCu7pXn7Ujj/NZX9ICo/V0dL9yBVm/EA7 5cbp+4cn8Nq1I1EmL36y52JlSIS/Yh6L++jQ15u3UIAq8t2o6CBCKE8WF5sj7EIZzot3 EuLgUWH6HCsxGkDD7cUYbM4BF7PhCiaGIFLDeVadavpRhCp+HeNcROW1dov1NRf46USi UHqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785305309; x=1785910109; h=in-reply-to:references:subject:cc:to:from:message-id:date :content-type:content-transfer-encoding:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=sK/uHPOHKOSpL2rOWH/430U5Wo4SeZ5sprv1VG2eCss=; b=nEHAtS/rIi/yxr8My/kI4x0hxHqOjdL2C57P3hL3NoqQow6LUXMxTHF2Izc2o/pBJr NoSkPwRMRoKla010hQvArPuKdIGBg5hHqtq6SazIpTAFqKWQUVET0QbZjSQTeT1n6n85 pk001EwualMaSzavqmW/6qm/+RBG2ttWcJ0bsjrF5krUjqk65NNKJpNn3o1DDMH/QtsL beXmPKq+r2I+4S18tEebmw3+uJetyx9NEh1q9gc4O7bSxhRSSCDIEpQkvbaR7f/IBvXX Dm897iCO701ABmDImevXXrqRDMIW0p0MUxiAwAHIfdIApwwwzSIlGiqKa9LXYI6w+c7w E6zQ== X-Forwarded-Encrypted: i=1; AHgh+RqHqEULiDv1xCvgP+mw+2SKFYM47IRoTlw7HUkvOI8wuqCrXxl//ecLWBvRNBuQkKAH58CJdTwx+vxj3fU=@vger.kernel.org X-Gm-Message-State: AOJu0YwhVYu+6FzxSuCRr6sWEBDEON9xtfq+9vOHV7gOsFd79qkIv8Ou //vsTSXmpsDfcVxT9FYpbR83fN8mk20iQCRjQZX6oI7a27oHU+xCT+nv9G5czqnfyqo= X-Gm-Gg: AR+sD12kT9AWQqzn9V83qnxMBLBjne6oIKHj3eiLH//u3x/myFSj9AN/FEJzBFmI6L/ IaSRlwJe9bnOZyvgaBHkBhrdRgRwUbpBQquY0pdECoXa1YjvwQf2mvvxFX+b+wFI0o/E0mgm5bu k2fUxYv+rKY+gITGLrgjQj8QYyVgc0E2eKYkK3Xjde6of/xgGXL2Oln/eHRJKRoAOuK5Sy8tOGm CD5uBmB6y5psbIwjDeuUJ1mDAJRzVAcyxUiFpQlAIwAEZUY7k7lxFO2yaL9L/3g6b4FwSN5C/pb VSqkD+HIdCJQCJwpIaMtOnB0PW+LU1xoqqw3nN7nS28VikCGp9ggbFzpmK52Go3RovJyXkhTiTG tS+HsPJoUY4Hk1J8zgp2EH/AW3bFEvrKgYCws5OBtUnsx3bWB6RG6aPbc2bKEwpbMZc2wbJ3qrG SjbU2Vwo5MfPxe694x58KGlbpQRbCLBHAzD2PB1j3wtswiJy8eAZO+7We9Oi4DNTen0qRuXLptj /B084Lpk9JmCvyBIg== X-Received: by 2002:a05:6a21:3983:b0:3c4:4555:2c22 with SMTP id adf61e73a8af0-3c8ba5d8955mr6437918637.40.1785305309034; Tue, 28 Jul 2026 23:08:29 -0700 (PDT) Received: from localhost (107-190-31-17.cpe.teksavvy.com. [107.190.31.17]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-cbdb55c5b7bsm408196a12.0.2026.07.28.23.08.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 28 Jul 2026 23:08:28 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Wed, 29 Jul 2026 02:08:27 -0400 Message-Id: From: "Emil Tsalapatis" To: "Junseo Lim" , "John Fastabend" , "Jakub Sitnicki" , "Jiayuan Chen" Cc: "David S. Miller" , "Eric Dumazet" , "Jakub Kicinski" , "Paolo Abeni" , "Simon Horman" , "Martin KaFai Lau" , , , , "Sechang Lim" Subject: Re: [PATCH bpf] bpf, sockmap: fix page_counter underflow in strparser SK_PASS X-Mailer: aerc 0.21.0-0-g5549850facc2 References: <20260723065244.186916-1-zirajs7@gmail.com> In-Reply-To: <20260723065244.186916-1-zirajs7@gmail.com> On Thu Jul 23, 2026 at 2:52 AM EDT, Junseo Lim wrote: > tcp_bpf_strp_read_sock() delays cleanup of SK_PASS bytes by > subtracting psock->ingress_bytes from the amount passed to > __tcp_cleanup_rbuf(). But when sk_psock_verdict_apply() queues the skb > directly through sk_psock_skb_ingress_self(), skb_set_owner_r() is called > unconditionally and charges the skb again. Can you specify in the commit that this is about sk_forward_alloc (AFAICT)? The eventual page_counter underflow is a second-order effect even if it's what eventually causes the crash. > > The duplicated charge is later released independently and can trigger a > page_counter underflow. > > Add a charge_skb argument to sk_psock_skb_ingress_self() and skip > skb_set_owner_r() only for the direct strparser SK_PASS path. Keep existi= ng > accounting for the other self-ingress caller and for non-strparser SK_PAS= S. > > Fixes: 36b62df5683c ("bpf: Fix wrong copied_seq calculation") > Signed-off-by: Junseo Lim > --- > Crash reproduced on Linux tree 94515f3a7d4256a5062176b7d6ed0471938cd51a > with KASAN, MEMCG, panic_on_warn=3D1, and oops=3Dpanic. Can you make a selftest out of the reproducer? > > Reproducer/log/config: https://gist.github.com/ZirAjs/16c95c89972ace73910= d9b5ac78a5807 > > The reproducer drives the strparser SK_PASS path until teardown reports: > > page_counter underflow > Workqueue: events sk_psock_destroy > Kernel panic - not syncing: kernel: panic_on_warn set ... > > net/core/skmsg.c | 29 +++++++++++++++++------------ > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/net/core/skmsg.c b/net/core/skmsg.c > index 2521b643fa05..17260681479a 100644 > --- a/net/core/skmsg.c > +++ b/net/core/skmsg.c > @@ -586,7 +586,8 @@ static int sk_psock_skb_ingress_enqueue(struct sk_buf= f *skb, > } > =20 > static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_b= uff *skb, > - u32 off, u32 len, bool take_ref); > + u32 off, u32 len, bool take_ref, > + bool charge_skb); > =20 > static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *= skb, > u32 off, u32 len) > @@ -595,12 +596,9 @@ static int sk_psock_skb_ingress(struct sk_psock *pso= ck, struct sk_buff *skb, > struct sk_msg *msg; > int err; > =20 > - /* If we are receiving on the same sock skb->sk is already assigned, > - * skip memory accounting and owner transition seeing it already set > - * correctly. > - */ > if (unlikely(skb->sk =3D=3D sk)) > - return sk_psock_skb_ingress_self(psock, skb, off, len, true); > + return sk_psock_skb_ingress_self(psock, skb, off, len, true, > + true); > msg =3D sk_psock_create_ingress_msg(sk, skb); > if (!msg) > return -EAGAIN; > @@ -618,12 +616,14 @@ static int sk_psock_skb_ingress(struct sk_psock *ps= ock, struct sk_buff *skb, > return err; > } > =20 > -/* Puts an skb on the ingress queue of the socket already assigned to th= e > - * skb. In this case we do not need to check memory limits or skb_set_ow= ner_r > - * because the skb is already accounted for here. > +/* Puts an skb on the ingress queue for psock->sk. > + * > + * When charge_skb is false, the direct strparser SK_PASS path keeps the= TCP > + * receive queue accounting in place and must not call skb_set_owner_r()= . > */ > static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_b= uff *skb, > - u32 off, u32 len, bool take_ref) > + u32 off, u32 len, bool take_ref, > + bool charge_skb) > { > struct sk_msg *msg =3D alloc_sk_msg(GFP_ATOMIC); > struct sock *sk =3D psock->sk; > @@ -631,7 +631,8 @@ static int sk_psock_skb_ingress_self(struct sk_psock = *psock, struct sk_buff *skb > =20 > if (unlikely(!msg)) > return -EAGAIN; > - skb_set_owner_r(skb, sk); > + if (charge_skb) > + skb_set_owner_r(skb, sk); Sashiko's right that skipping this leaves the skb half-configured. Since the point is to not double-count sk_forward_alloc but we still have to account truesize, can you try something like: if (!skb_charge) sk_rmem_schedule(sk, skb, 0); skb_set_owner_r(skb, sk); to just ajust the sk_forward_alloc? pw-bot: cr > =20 > /* This is used in tcp_bpf_recvmsg_parser() to determine whether the > * data originates from the socket's own protocol stack. No need to > @@ -1017,6 +1018,8 @@ static int sk_psock_verdict_apply(struct sk_psock *= psock, struct sk_buff *skb, > * retrying later from workqueue. > */ > if (skb_queue_empty(&psock->ingress_skb)) { > + bool charge_skb =3D true; > + > len =3D skb->len; > off =3D 0; > if (skb_bpf_strparser(skb)) { > @@ -1024,8 +1027,10 @@ static int sk_psock_verdict_apply(struct sk_psock = *psock, struct sk_buff *skb, > =20 > off =3D stm->offset; > len =3D stm->full_len; > + charge_skb =3D false; > } > - err =3D sk_psock_skb_ingress_self(psock, skb, off, len, false); > + err =3D sk_psock_skb_ingress_self(psock, skb, off, len, > + false, charge_skb); > } > if (err < 0) { > spin_lock_bh(&psock->ingress_lock);