From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-180.mta0.migadu.com (out-180.mta0.migadu.com [91.218.175.180]) (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 789465FB95 for ; Mon, 2 Jun 2025 11:04:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748862298; cv=none; b=eTz7FfDSpqRBOKko+lSaAIdyrSKcHriQ7WSfWvyJ5y6x87jT+6NR3HEFs5r9+24W+SOF6jxrXO3e0FzBeH1Mx3jAmfa2SWcVnUyhl3jP99uTirI0c3MOzNzdteKsDkE2t56lVQujVCHzlXHFurCnb45O5r6XpxkvLexu91NU1lc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1748862298; c=relaxed/simple; bh=A4OkNO6pKJdoJr7vYx9FCXoetIQb+v3bG4umPUpfy0U=; h=MIME-Version:Date:Content-Type:From:Message-ID:Subject:To:Cc: In-Reply-To:References; b=alr4kpTT33kz2qOXoq4opzmpq0td9hj9v5GEN6Oq/gvhKJewCxNImKIsJfUwT08bdVKWHb7c0YuIBddHLoQq4CfmlN1xBZAkRGI7whnCvKbOGbVHRMR5s8/4VMACtnimnwSK6GEfooXTn30SVWsOQ+Ewf13UpjHIyWnUOR2hnmg= 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=RJvGo5Eu; arc=none smtp.client-ip=91.218.175.180 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="RJvGo5Eu" Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1748862292; 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=GgzGn7mX9LPHdW6wIq04NEoY4o/2aPy+ycvjiVMwLpA=; b=RJvGo5Eu4br18T4wfwZrZxPFmIVPvINsrBo79u62AgI5wKOCaCRHE5yQvXLFeAq1u04q7N FBhRi1ecNlz/GAEFqwRlNVaOJYJYYo9zPrbLJtzhOBJSXiooO43G0A3vV6B3AcV6RBQC9Y LKGiZjm0w1tEwp6JhnEU7ycN1A/x6fY= Date: Mon, 02 Jun 2025 11:04:50 +0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: "Jiayuan Chen" Message-ID: TLS-Required: No Subject: Re: [PATCH bpf-next v1 1/2] bpf,ktls: Fix data corruption when using bpf_msg_pop_data() in ktls To: "Cong Wang" Cc: bpf@vger.kernel.org, "Boris Pismenny" , "John Fastabend" , "Jakub Kicinski" , "David S. Miller" , "Eric Dumazet" , "Paolo Abeni" , "Simon Horman" , "Andrii Nakryiko" , "Eduard Zingerman" , "Mykola Lysenko" , "Alexei Starovoitov" , "Daniel Borkmann" , "Martin KaFai Lau" , "Song Liu" , "Yonghong Song" , "KP Singh" , "Stanislav Fomichev" , "Hao Luo" , "Jiri Olsa" , "Shuah Khan" , "Ihor Solodrai" , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org In-Reply-To: References: <20250523131915.19349-1-jiayuan.chen@linux.dev> <20250523131915.19349-2-jiayuan.chen@linux.dev> X-Migadu-Flow: FLOW_OUT 2025/5/30 02:16, "Cong Wang" =E5=86=99=E5=88= =B0: >=20 >=20On Fri, May 23, 2025 at 09:18:58PM +0800, Jiayuan Chen wrote: >=20 >=20>=20 >=20> When sending plaintext data, we initially calculated the correspond= ing > >=20 >=20> ciphertext length. However, if we later reduced the plaintext data= length > >=20 >=20> via socket policy, we failed to recalculate the ciphertext length. > >=20 >=20>=20=20 >=20>=20 >=20> This results in transmitting buffers containing uninitialized data= during > >=20 >=20> ciphertext transmission. > >=20 >=20>=20=20 >=20>=20 >=20> This causes uninitialized bytes to be appended after a complete > >=20 >=20> "Application Data" packet, leading to errors on the receiving end = when > >=20 >=20> parsing TLS record. > >=20 >=20>=20=20 >=20>=20 >=20> Fixes: d3b18ad31f93 ("tls: add bpf support to sk_msg handling") > >=20 >=20> Reported-by: Cong Wang > >=20 >=20> Signed-off-by: Jiayuan Chen > >=20 >=20> --- > >=20 >=20> net/tls/tls_sw.c | 15 +++++++++++++++ > >=20 >=20> 1 file changed, 15 insertions(+) > >=20 >=20>=20=20 >=20>=20 >=20> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c > >=20 >=20> index fc88e34b7f33..b23a4655be6a 100644 > >=20 >=20> --- a/net/tls/tls_sw.c > >=20 >=20> +++ b/net/tls/tls_sw.c > >=20 >=20> @@ -872,6 +872,21 @@ static int bpf_exec_tx_verdict(struct sk_msg = *msg, struct sock *sk, > >=20 >=20> delta =3D msg->sg.size; > >=20 >=20> psock->eval =3D sk_psock_msg_verdict(sk, psock, msg); > >=20 >=20> delta -=3D msg->sg.size; > >=20 >=20> + > >=20 >=20> + if ((s32)delta > 0) { > >=20 >=20> + /* It indicates that we executed bpf_msg_pop_data(), > >=20 >=20> + * causing the plaintext data size to decrease. > >=20 >=20> + * Therefore the encrypted data size also needs to > >=20 >=20> + * correspondingly decrease. We only need to subtract > >=20 >=20> + * delta to calculate the new ciphertext length since > >=20 >=20> + * ktls does not support block encryption. > >=20 >=20> + */ > >=20 >=20> + if (!WARN_ON_ONCE(!ctx->open_rec)) { > >=20 >=20 > I am wondering if we need to WARN here? Because the code below this >=20 >=20handles it gracefully: >=20 Hi=20Cong The ctx->open_rec is freed after a TLS record is processed (regardless of whether the redirect check passes or triggers a redirect). The 'if (rec)' check in the subsequent code you print is indeed designed to handle the expected lifecycle state of open_rec. But the code path I modified should never see a NULL open_rec under norma= l operation As this is a bug fix, I need to ensure the fix itself doesn't create new issues.=20 Thanks. >=20 931 bool reset_eval =3D !ctx->open_rec; >=20 >=20 932=20 >=20 > 933 rec =3D ctx->open_rec; >=20 >=20 934 if (rec) { >=20 >=20 935 msg =3D &rec->msg_plaintext; >=20 >=20 936 if (!msg->apply_bytes) >=20 >=20 937 reset_eval =3D true; >=20 >=20 938 } >=20 >=20 939 if (reset_eval) { >=20 >=20 940 psock->eval =3D __SK_NONE; >=20 >=20 941 if (psock->sk_redir) { >=20 >=20 942 sock_put(psock->sk_redir); >=20 >=20 943 psock->sk_redir =3D NULL; >=20 >=20 944 } >=20 >=20 945 } >=20 >=20Thanks for fixing it! >=20 >=20Cong >