From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) (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 8C5644ADDA3 for ; Tue, 9 Jun 2026 18:21:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781029317; cv=none; b=HxmWXw+qXQGmhiXA0J3S5AsPsTdtB7o/h+F0pHcENlsK9aJ24mytDZq7fAFGayiCIjpR8JvvUqFcSIzzwFaVOEadST8eHxpt+L4rvuqbg+3zjwEHh264Rl382o3+YzzQvbvXXCIPwFh71U0ZqYMySA4jE7OFWxsAc9E1poyeLZI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781029317; c=relaxed/simple; bh=MDqw6E1NFQvJRAPqDEem1t57vShB0cBoV13q5Qxc330=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WqhBFnLXAGPyHVzJTQFhozIJcyWhNVKitEqbyRgqwhp/31cvA27JdBsTZoRPRcolc7DpAGCXFl3+NWtp9o3+blfJLMic5XuG9tQKp65g+2HKZmAOqNvVkAOqGBU/LpsB2mFkC0swXSQKHmr5MdgdbU1YDFYpitlCagJGnqZV2EA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=RawhVVrO; arc=none smtp.client-ip=209.85.216.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RawhVVrO" Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-36b9b15af73so5522054a91.0 for ; Tue, 09 Jun 2026 11:21:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1781029315; x=1781634115; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=qcqjUQA9kv2/3qirEOddNlpBhe6IWcklo76jpxd9AfA=; b=RawhVVrOg/OQSQo4NeNprd+5IGEaO6LrXYvqyLhcdMI1qX8tsw7ujp20lc2/evpMFv Af8HzSQKo5wbE5yP7giG040BEh4VsgoXFF5eyBu4zctf8fAOhNwpIIxI30vPOFtPyapG OyDEV0ezgmRmVDj671vbbzmMxRqVUx9Gd39OmT0WU7DktyUqDpwYfg5YxnB1wssIDQv+ UxynomBMz4uqNJVG7IUY/a4hcuq2MhA04fHbVfaIfJrv8kLPMzWizLpGxGl3KMtEEShJ sPl/5n2gRaSB5r/RTQ0DloP8xmbxrB9IYTTQ7zC068rdwk/Gz0MkaHq/Rex9jobGNBIJ 1TPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781029315; x=1781634115; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=qcqjUQA9kv2/3qirEOddNlpBhe6IWcklo76jpxd9AfA=; b=frSzDJHiJR8DMADCf92XK18sggBCilzMyxA9O4IXLVDoL6AkhknZM1rvVEIHKbLk1v 9N+wz+bDsVxWYz2sr5YRJpULqmaWvT5VwHZBDwJeGGhaBIKJRNm/zjDn87qDHH0lbAR2 y6WZIF25fjudTlH7BPPaZz+r8HGFX2U/qMm2qDS4WZVkjFA8v1MEZ85fz4aiZxLMy7uy e5XzR10uXvIctBTlXszoeEV9v1xhUQvde+HLJGhibXrsq5izjxkISO6/1q/4p+FYjiJY sK6ibF9MmX6B9ZWv76yZjsm1kg09ClRDfS8YieC0iaRLmM4LEhg3N7twGdl/iVz7PfP/ v9HQ== X-Forwarded-Encrypted: i=1; AFNElJ8rnNenWddAHbi1riBpTY0U+lf1rgzGGNjHdoxlQrlV9MViynXSo0Hce4hzOxI6DuQ58gXVJlgNfGR5fuo=@vger.kernel.org X-Gm-Message-State: AOJu0YyEOcbB8KxC1w94RtD+SEcTwqFsz8kccV3DY+btXxu3RapQ+oWG TtvRrY75V+FRW0BGSDuKi3gmYIf18/fEBxU9sdEnsOFsifuGp+UbLVY4 X-Gm-Gg: Acq92OFLDnInSSMPPdS35RQedgWiJ/Dq2121EQkVXZyt41HfhV/9hUqAGh1knZvvSw4 nuj9RMoAg368SSu/Qiq6wIaEJNMJW9c7aHIQzmcQprU69sMgRWEcGP9C1oj+TXSWP4dzaPegqSK 4lv+E8P19zsYi4buV8CU3RP+zGPXsNhDxeIkfrFQViylL+Iq7yC4MCR9AHk0DxvFRi8ESPd32jf 9npd3hFH3smq3c2z2jUEJ8mRtaA1rOj1qXdxZ9v1aqvWkzfziQBNXcol8ocqB8pAimtDCw54f4b JjFy6zKX3kk5MgRAB+SB+NX3HZFkQZkWJIgQwBTTY20851oh/wzZzUByPpJnRGgAIMyRB+fwkvv kF1KDCt+oEaPVqDrcX1Qf4f5Eknwq/aZZXirB/EJVK97EIMJE6XL5BYTO3Pq2HpYqXsiBjNj0Nc pk31MrJPgnSZvNpt4cdutVarSGY1xz+elLNOFxiu3Im0Yx1X6M7rIdv80kBcMklx8f X-Received: by 2002:a17:90b:5109:b0:372:94b9:76d8 with SMTP id 98e67ed59e1d1-37294b97715mr15579831a91.6.1781029314769; Tue, 09 Jun 2026 11:21:54 -0700 (PDT) Received: from devvm29614.prn0.facebook.com ([2a03:2880:ff:8::]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2c164fa072asm218684505ad.34.2026.06.09.11.21.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jun 2026 11:21:54 -0700 (PDT) Date: Tue, 9 Jun 2026 11:21:52 -0700 From: Bobby Eshleman To: Sechang Lim Cc: John Fastabend , Jakub Sitnicki , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , netdev@vger.kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH bpf] bpf, sockmap: fix use-after-free when the stream parser resizes the skb Message-ID: References: <20260609112316.3685738-1-rhkrqnwk98@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260609112316.3685738-1-rhkrqnwk98@gmail.com> On Tue, Jun 09, 2026 at 11:23:03AM +0000, Sechang Lim wrote: > sk_psock_strp_parse() runs the BPF_PROG_TYPE_SK_SKB stream-parser program to > find the length of the next message. strparser assembles a message out of > several received skbs by chaining them onto the head's frag_list and recording > where to append the next one in strp->skb_nextp: > > *strp->skb_nextp = skb; > strp->skb_nextp = &skb->next; > > and then calls the parser on the head: > > len = (*strp->cb.parse_msg)(strp, head); > > The parser is only meant to inspect the skb, but the program may call > bpf_skb_change_tail() -- or the sibling bpf_skb_pull_data(), > bpf_skb_change_head(), bpf_skb_adjust_room(), all allowed for SK_SKB. Once the > head carries a frag_list these go > > ... -> skb_ensure_writable -> pskb_may_pull -> __pskb_pull_tail > > and __pskb_pull_tail() frees the frag_list skbs that strparser still tracks > through skb_nextp: > > while ((list = skb_shinfo(skb)->frag_list) != insp) { > skb_shinfo(skb)->frag_list = list->next; > consume_skb(list); > } > > strp->skb_nextp now points into a freed sk_buff. The next segment of the same > message arrives in __strp_recv(), which links it with *strp->skb_nextp = skb, > an 8-byte write into the freed skb. The free and the write happen in different > __strp_recv() calls, so the message has to span at least three segments before > it triggers. > > BUG: KASAN: slab-use-after-free in __strp_recv+0x447/0xda0 > Write of size 8 at addr ffff88810db86140 by task repro/349 > > Call Trace: > > __strp_recv+0x447/0xda0 > __tcp_read_sock+0x13d/0x590 > tcp_bpf_strp_read_sock+0x195/0x320 > strp_data_ready+0x267/0x340 > sk_psock_strp_data_ready+0x1ce/0x350 > tcp_data_queue+0x1364/0x2fd0 > tcp_rcv_established+0xe07/0x1640 > [...] > > Allocated by task 349: > skb_clone+0x17b/0x210 > __strp_recv+0x2c3/0xda0 > __tcp_read_sock+0x13d/0x590 > [...] > > Freed by task 349: > kmem_cache_free+0x150/0x570 > __pskb_pull_tail+0x57b/0xc20 > skb_ensure_writable+0x236/0x260 > __bpf_skb_change_tail+0x1d4/0x590 > sk_skb_change_tail+0x2a/0x40 > bpf_prog_1b285dcd6c41373e+0x27/0x30 > bpf_prog_run_pin_on_cpu+0xf3/0x260 > sk_psock_strp_parse+0x118/0x1e0 > __strp_recv+0x4f6/0xda0 > [...] > > The same resize also leaves the head's length inconsistent with its frags, so > a later __pskb_pull_tail() can instead hit the BUG_ON(skb_copy_bits(...)) in > net/core/skbuff.c. > > Run the parser on a private clone of the head whenever the message spans more > than one skb, so a resizing helper can only touch the clone and strparser's > head and skb_nextp stay valid. Single-skb messages have no frag_list and are > still parsed in place. > > Fixes: 8a31db561566 ("bpf: add access to sock fields and pkt data from sk_skb programs") > Signed-off-by: Sechang Lim > --- > net/core/skmsg.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/net/core/skmsg.c b/net/core/skmsg.c > index e1850caf1a71..d5b10f9b0ba8 100644 > --- a/net/core/skmsg.c > +++ b/net/core/skmsg.c > @@ -1146,14 +1146,30 @@ static int sk_psock_strp_parse(struct strparser *strp, struct sk_buff *skb) > struct bpf_prog *prog; > int ret = skb->len; > > - rcu_read_lock(); > + guard(rcu)(); I'd probably not mix this rcu change into this patch, though I understand it drops an unlock() call in the new err return. > prog = READ_ONCE(psock->progs.stream_parser); > if (likely(prog)) { > - skb->sk = psock->sk; > - ret = bpf_prog_run_pin_on_cpu(prog, skb); > - skb->sk = NULL; > + struct sk_buff *parse_skb = skb; > + > + /* > + * strparser chains the message skbs through skb->frag_list and > + * keeps a pointer into that list in strp->skb_nextp. The parser > + * program may call bpf_skb_change_tail() and friends, which go > + * through __pskb_pull_tail() and free the frag_list skbs that > + * strparser still tracks. Run the program on a clone when a > + * frag_list is present so it cannot drop frags strparser owns. > + */ > + if (skb_has_frag_list(skb)) { > + parse_skb = skb_clone(skb, GFP_ATOMIC); > + if (!parse_skb) > + return -ENOMEM; I wonder if this could be further gated by prog->aux->changes_pkt_data? One possible issue with adding an allocation here and returning -ENOMEM is that this will shutdown stream parsing permanently for this sk when under memory pressure, which is new behavior. Maybe, if we return 0, it looks like the caller's accounting stays true by taking the "needs more header" path, and then we can try again from the same place on the next data_ready, and so be resilient even under memory pressure? I guess the trade-off being that their may not come another data_ready to cause progress... not sure which is better. Maybe strp_start_timer could be used here too? Best, Bobby