From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-183.mta0.migadu.com (out-183.mta0.migadu.com [91.218.175.183]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D68DB3839BA; Tue, 9 Jun 2026 13:55:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.183 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781013324; cv=none; b=cHaZnnLU+a0KVAgTOSrovtFogvDps5PrRop5Hzydyfeu/SPtRkvZ6IJWqnScNmw74Nr4OG6AEGqdo1bTMHFhnmzrg7DonWp+001vZLucNviQM86+etuobwVaeFCWCheviLCKT4RyA0XvnbgozyJqEAo17iZWka4nd+VToA2OVMk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781013324; c=relaxed/simple; bh=7B0VErhoO4IBLdB2pAQxOtd59aX4STChOKYtOvn3Bos=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=To04pHX6fC7x0bc1xquG/ywnrxwgge1gAfNfayosFOwGrSJnN8t94EKXhs7KxbqyUgx2gy+2V/ns6a7BRdp7FmzKvlYFizcM6bKvl7o/9jJYuoEtiQ7Sur8Z9mCelrWb2fzLUUW2HnKKu7LaI3j6DFbDuzJXgFDQtjudzX+/NTc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=htAaPuM4; arc=none smtp.client-ip=91.218.175.183 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="htAaPuM4" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1781013310; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WBshCEsRUNVy8WU5f/Zn3mDRm2/wce9vXr1J5RuHeR4=; b=htAaPuM4fnOFbMYZaI1pGxilzz4ejjV1WNh/zLiXjozIaHA7pqxaYOrEVTUAvTsDuKisbl YHSQsCs6VThLtTH3fgNdDtV0MZpILgQvNTaN150sa1mexygstjuzMdA3Q5zPIaIAc6EOge 5NWO7RzaD7l1+TdJxBm7fV77NN3rSGk= Date: Tue, 9 Jun 2026 21:54:38 +0800 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH bpf v8 1/2] net: Validate protocol in skb_steal_sock() for BPF-assigned sockets To: Kuniyuki Iwashima , Alexei Starovoitov Cc: Alexei Starovoitov , bpf , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Willem de Bruijn , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Kumar Kartikeya Dwivedi , Song Liu , Yonghong Song , Jiri Olsa , Shuah Khan , Joe Stringer , Network Development , LKML , "open list:KERNEL SELFTEST FRAMEWORK" References: <20260608125846.157004-1-jiayuan.chen@linux.dev> <20260608125846.157004-2-jiayuan.chen@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Jiayuan Chen In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT On 6/9/26 6:34 AM, Kuniyuki Iwashima wrote: > On Mon, Jun 8, 2026 at 3:16 PM Alexei Starovoitov > wrote: >> On Mon Jun 8, 2026 at 2:35 PM PDT, Kuniyuki Iwashima wrote: >>>> btw is earlier sashiko TOCTOU concern real? >>> Yes, the selftest in patch 2 should reproduce null-ptr-deref. >>> (I tested one in a prior version) >>> >>> >>>> iph->protocol = IPPROTO_TCP; >>>> bpf_sk_assign_tcp_reqsk(udp_skb, tcp_sk); >>>> iph->protocol = IPPROTO_UDP; >>>> >>>> as far as I can tell past bpf_sk_assign_tcp_reqsk() >>>> the networking stack only looks at skb->protocol, >>>> so modifying iph->protocol will only mess up >>>> things on the wire (if this packet goes out). >>> ip_rcv_finish_core() uses iph->protocol for early demux >>> and ip_local_deliver_finish() also uses it for demux. >> ok. Another idea... we can keep the checks on bpf side and >> teach the verifier to invalidate packet pointers at bpf_sk_assign_tcp_reqsk() >> and introduce new concept of "readonly skb". >> So after invalidation the reloaded sbk->data will be read only. > Yes, this is exactly what I had in mind in the other thread :) > > ---8<--- > On top of L4 validation in bpf_sk_assign() and bpf_sk_assign_tcp_reqsk(), > can't we mark such an skb immutable after the helpers and catch > subsequent writes to skb->data on the verifier ? > ---8<--- > > >> There is no such thing today. The verifier only understands PTR_TO_PACKET >> as read/write. So it's not easy, but probably good thing to have >> for this and other cases where bpf prog shouldn't touch the packet >> once it called some kfunc/helper. > This would be useful and we could (re)move some validations from > the fast path. Hi Kuniyuki and Alexei, Thanks both for the discussion, it makes sense to me. I'll go back to fixing this in the helper itself, like the previous version did. Then, as a longer-term and more general solution, I'll try look into marking the skb as immutable after these helpers so the verifier can reject any later writes to the packet.