Message ID | 20231122214447.675768-1-jannh@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6358:6f03:b0:164:83eb:24d7 with SMTP id r3csp1520511rwn; Wed, 22 Nov 2023 13:45:09 -0800 (PST) X-Google-Smtp-Source: AGHT+IHogml5W2U48xwYKnMDMbwBUWvZ8/e1QZVw2zVrlzt+8vtkN66PX9G36V0uKRApgKEDKGFl X-Received: by 2002:a17:902:ced1:b0:1cc:36fb:22ae with SMTP id d17-20020a170902ced100b001cc36fb22aemr1196720plg.2.1700689509526; Wed, 22 Nov 2023 13:45:09 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700689509; cv=none; d=google.com; s=arc-20160816; b=nwufEcHKIV+sIo3/XNVTr5pvUSJ6Iz5AoMt72xTqTRXXvUTilUKLywCnB/5MLDQafQ PRBLOWgqnxBXXxxqxTBHe5uj6rnXDktOhsVx/fL5UahUzPNLKOa7VCM/sMPflcXvPxEW p5q520l+l8Oy58ojO6KEqtftBbYvCygluT8CMn+nPr7mcjjloqsS23C1HxZCtITtfkmc tIcQ12EUwntu7TKHGS/4CJW4h6gFFum+eq5hY0HOwBuHYBId24rVa3K2wc7+ePNvFhuI OgLYDR/riy/qnosPwnXYjz0PP5rT6LDpw+MVdo1nr8iYwU3ULBZM3uQgUrElzl5xRgNE 7kDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=nXiWhRqFrWoSNtAFmkHRO3t9Ruwff9Gl/eAaWgquIiw=; fh=FALFMpcazHLdNwVgmoJx2hGOSLYlSxZGeTZm15KTbDI=; b=zX/HxQa5wF0DqtewI6XL4B0r4SP4Ls2aleCd9kb2vT5Uxw1lwNsD8SbTok2TzizbQG GxtVvoXrnlEzVESGKOyyS8dBlmvMDU0U3Og9j4jp1upKsQkOUk7YdJJ8Of0jdhkke05Y edNeTLcS9/0O6ZwU8ZxxlfXI0uahY3eFePdCXKI6ynA3wg6gA+mWHFZu1v5ju8FkIAri HEh75Slwa8vjDjBmejfKoDnMFHPX0TUf+A4aYB6byfdBWrV1jIU0zJudvAMpdNwq0M/5 9ftBaKXR548Q2cDNFp/6D9hLWv6IcrJ/CyjWDlAHSxiDTiaYCH/mUHUGBcENxF/c9Cnh f/kQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=MZYU6+Fw; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id iz17-20020a170902ef9100b001bb3bcd05bbsi250970plb.471.2023.11.22.13.45.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Nov 2023 13:45:09 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=MZYU6+Fw; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id E251380A5F90; Wed, 22 Nov 2023 13:45:07 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344432AbjKVVpE (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Wed, 22 Nov 2023 16:45:04 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52908 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1344312AbjKVVpC (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 22 Nov 2023 16:45:02 -0500 Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C56E01BE for <linux-kernel@vger.kernel.org>; Wed, 22 Nov 2023 13:44:58 -0800 (PST) Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-408c6ec1fd1so4115e9.1 for <linux-kernel@vger.kernel.org>; Wed, 22 Nov 2023 13:44:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1700689497; x=1701294297; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=nXiWhRqFrWoSNtAFmkHRO3t9Ruwff9Gl/eAaWgquIiw=; b=MZYU6+FwDpBJ6xo/We22FfLNh9FSQ9OoFhPVwTcojSXrT4Ftp9AqlcnWAfNmpcIr9K V9jZa0+1o205wL1nn/7di5gW7qfzBgaK7kfvFbvKVh5WyWizYTjVcBO/mMAczdYCdGuc 1nKdjyS7RvhOij8CAByT3z39g1PfqZD9SM9q96esBHOWV05EJmW69iOnUSz6B7eLtlwX P3E+wMiSzcK91KwTHnBXYcXwue+qaXf14m8M8fQQin74OtQURnHfN6z78jU74naoOSPQ yllW3hBiopQWAJ0YfBOBUrVXzsuC32qIA4B6/iZzza4HJzPSskHO4yMRKtxJ3NVZ6N1E fZPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700689497; x=1701294297; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=nXiWhRqFrWoSNtAFmkHRO3t9Ruwff9Gl/eAaWgquIiw=; b=LTs3b28wT6YiFQzcaULq0uRFMiI5banlpK1lPVudXIsOPKP7hrC3f33VeEC89+5qlN YvPYctcodSJ4+OjW/vKbv95pre3bNI9T5fEPDatrTdu12gTd4mfZSyr3I6VLd3KYWS+V x7llqnhs7pvep82pmwbx2vbPcjBvRm4vm/MrfT9S7Sptb3C9S16tAyWpS3z/pS6RFJIO eYmPTquRj6sLMf6QIYlNbWb3bdUU/MfFInQn9Clqzm+M3qsAIe8e/sguBjDIzWVtpjzq 9+Bgt/Mx301fGY4MkuxUct1molm41cqJsz+y7HOQ817h0YHKRusb8ficIV/aigDFQY3b JiqQ== X-Gm-Message-State: AOJu0Yy/Te8YQbZeyjiU+uBxEpIgylB9hJa/IqH4dfPY60HSe39kzT4Z ZhVbrb65kLqK87/yw2HOjDFxgg== X-Received: by 2002:a05:600c:4e0a:b0:40a:4c7d:f300 with SMTP id b10-20020a05600c4e0a00b0040a4c7df300mr204704wmq.6.1700689496992; Wed, 22 Nov 2023 13:44:56 -0800 (PST) Received: from localhost ([2a00:79e0:9d:4:309f:a860:c565:1fc7]) by smtp.gmail.com with ESMTPSA id k27-20020a5d525b000000b0032ddf2804ccsm375746wrc.83.2023.11.22.13.44.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Nov 2023 13:44:54 -0800 (PST) From: Jann Horn <jannh@google.com> To: Boris Pismenny <borisp@nvidia.com>, John Fastabend <john.fastabend@gmail.com>, Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net> Cc: David Howells <dhowells@redhat.com>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net] tls: fix NULL deref on tls_sw_splice_eof() with empty record Date: Wed, 22 Nov 2023 22:44:47 +0100 Message-ID: <20231122214447.675768-1-jannh@google.com> X-Mailer: git-send-email 2.43.0.rc1.413.gea7ed67945-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-17.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 22 Nov 2023 13:45:08 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783302203002041536 X-GMAIL-MSGID: 1783302203002041536 |
Series |
[net] tls: fix NULL deref on tls_sw_splice_eof() with empty record
|
|
Commit Message
Jann Horn
Nov. 22, 2023, 9:44 p.m. UTC
syzkaller discovered that if tls_sw_splice_eof() is executed as part of
sendfile() when the plaintext/ciphertext sk_msg are empty, the send path
gets confused because the empty ciphertext buffer does not have enough
space for the encryption overhead. This causes tls_push_record() to go on
the `split = true` path (which is only supposed to be used when interacting
with an attached BPF program), and then get further confused and hit the
tls_merge_open_record() path, which then assumes that there must be at
least one populated buffer element, leading to a NULL deref.
It is possible to have empty plaintext/ciphertext buffers if we previously
bailed from tls_sw_sendmsg_locked() via the tls_trim_both_msgs() path.
tls_sw_push_pending_record() already handles this case correctly; let's do
the same check in tls_sw_splice_eof().
Fixes: df720d288dbb ("tls/sw: Use splice_eof() to flush")
Cc: stable@vger.kernel.org
Reported-by: syzbot+40d43509a099ea756317@syzkaller.appspotmail.com
Signed-off-by: Jann Horn <jannh@google.com>
---
Note: syzkaller did not find a reliable reproducer for this; I wrote
my own reproducer that reliably hits the crash on an unpatched kernel,
and I've verified that the crash goes away with the patch applied.
net/tls/tls_sw.c | 3 +++
1 file changed, 3 insertions(+)
base-commit: 98b1cc82c4affc16f5598d4fa14b1858671b2263
Comments
On Wed, Nov 22, 2023 at 10:44 PM Jann Horn <jannh@google.com> wrote: > syzkaller discovered that if tls_sw_splice_eof() is executed as part of > sendfile() when the plaintext/ciphertext sk_msg are empty, the send path > gets confused because the empty ciphertext buffer does not have enough > space for the encryption overhead. This causes tls_push_record() to go on > the `split = true` path (which is only supposed to be used when interacting > with an attached BPF program), and then get further confused and hit the > tls_merge_open_record() path, which then assumes that there must be at > least one populated buffer element, leading to a NULL deref. Ah, and in case you're looking for the corresponding syzkaller report, you can find that at <https://lore.kernel.org/all/000000000000347a250608e8a4d1@google.com/T/>.
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 22 Nov 2023 22:44:47 +0100 you wrote: > syzkaller discovered that if tls_sw_splice_eof() is executed as part of > sendfile() when the plaintext/ciphertext sk_msg are empty, the send path > gets confused because the empty ciphertext buffer does not have enough > space for the encryption overhead. This causes tls_push_record() to go on > the `split = true` path (which is only supposed to be used when interacting > with an attached BPF program), and then get further confused and hit the > tls_merge_open_record() path, which then assumes that there must be at > least one populated buffer element, leading to a NULL deref. > > [...] Here is the summary with links: - [net] tls: fix NULL deref on tls_sw_splice_eof() with empty record https://git.kernel.org/netdev/net/c/53f2cb491b50 You are awesome, thank you!
Jann Horn wrote: > On Wed, Nov 22, 2023 at 10:44 PM Jann Horn <jannh@google.com> wrote: > > syzkaller discovered that if tls_sw_splice_eof() is executed as part of > > sendfile() when the plaintext/ciphertext sk_msg are empty, the send path > > gets confused because the empty ciphertext buffer does not have enough > > space for the encryption overhead. This causes tls_push_record() to go on > > the `split = true` path (which is only supposed to be used when interacting > > with an attached BPF program), and then get further confused and hit the > > tls_merge_open_record() path, which then assumes that there must be at > > least one populated buffer element, leading to a NULL deref. > > Ah, and in case you're looking for the corresponding syzkaller report, > you can find that at > <https://lore.kernel.org/all/000000000000347a250608e8a4d1@google.com/T/>. I'm a bit slow, but looks good to me as well. Thanks a lot. Acked-by: John Fastabend <john.fastabend@gmail.com>
Jann Horn <jannh@google.com> wrote:
> + /* same checks as in tls_sw_push_pending_record() */
Wouldn't it be better to say what you're checking rather than referring off to
another function that might one day disappear or be renamed?
David
On Mon, Nov 27, 2023 at 10:04 AM David Howells <dhowells@redhat.com> wrote: > Jann Horn <jannh@google.com> wrote: > > > + /* same checks as in tls_sw_push_pending_record() */ > > Wouldn't it be better to say what you're checking rather than referring off to > another function that might one day disappear or be renamed? Hm, maybe? My thought was that since this is kind of a special version of what tls_sw_push_pending_record() does, it's clearer to refer to sort of the canonical version of these checks. And if that ever disappears or gets renamed or whatever, and someone misses the comment, you'll still have git history to look at. And if, in the future, someone decides to add more checks to tls_sw_push_pending_record() for whatever reason, commenting it this way will make it clearer that tls_sw_splice_eof() could potentially require the same checks.
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index a78e8e722409..316f76187962 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1232,11 +1232,14 @@ void tls_sw_splice_eof(struct socket *sock) lock_sock(sk); retry: + /* same checks as in tls_sw_push_pending_record() */ rec = ctx->open_rec; if (!rec) goto unlock; msg_pl = &rec->msg_plaintext; + if (msg_pl->sg.size == 0) + goto unlock; /* Check the BPF advisor and perform transmission. */ ret = bpf_exec_tx_verdict(msg_pl, sk, false, TLS_RECORD_TYPE_DATA,