Received-SPF: pass (google.com: domain of linux-nfs+bounces-788-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <CAHk-=wgJz36ZE66_8gXjP_TofkkugXBZEpTr_Dtc_JANsH1SEw@mail.gmail.com>
References: <CAHk-=wgJz36ZE66_8gXjP_TofkkugXBZEpTr_Dtc_JANsH1SEw@mail.gmail.com> <1843374.1703172614@warthog.procyon.org.uk> <20231223172858.GI201037@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>,
    Edward Adam Davis <eadavis@qq.com>
Cc: dhowells@redhat.com, Simon Horman <horms@kernel.org>,
    Markus Suvanto <markus.suvanto@gmail.com>,
    Jeffrey E Altman <jaltman@auristor.com>,
    Marc Dionne <marc.dionne@auristor.com>,
    Wang Lei <wang840925@gmail.com>, Jeff Layton <jlayton@redhat.com>,
    Steve French <smfrench@gmail.com>,
    Jarkko Sakkinen <jarkko@kernel.org>,
    "David S. Miller" <davem@davemloft.net>,
    Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
    Paolo Abeni <pabeni@redhat.com>, linux-afs@lists.infradead.org,
    keyrings@vger.kernel.org, linux-cifs@vger.kernel.org,
    linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org,
    netdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
    linux-kernel@vger.kernel.org
Subject: [PATCH] keys, dns: Fix missing size check of V1 server-list header
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2592944.1703376169.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 24 Dec 2023 00:02:49 +0000
Message-ID: <2592945.1703376169@warthog.procyon.org.uk>

Hi Linus, Edward,

Here's Linus's patch dressed up with a commit message.  I would marginally
prefer just to insert the missing size check, but I'm also fine with Linus=
's
approach for now until we have different content types or newer versions.

Note that I'm not sure whether I should require Linus's S-o-b since he mad=
e
modifications or whether I should use a Codeveloped-by line for him.

David
---
From: Edward Adam Davis <eadavis@qq.com>

keys, dns: Fix missing size check of V1 server-list header

The dns_resolver_preparse() function has a check on the size of the payloa=
d
for the basic header of the binary-style payload, but is missing a check
for the size of the V1 server-list payload header after determining that's
what we've been given.

Fix this by getting rid of the the pointer to the basic header and just
assuming that we have a V1 server-list payload and moving the V1 server
list pointer inside the if-statement.  Dealing with other types and
versions can be left for when such have been defined.

This can be tested by doing the following with KASAN enabled:

        echo -n -e '\x0\x0\x1\x2' | keyctl padd dns_resolver foo @p

and produces an oops like the following:

        BUG: KASAN: slab-out-of-bounds in dns_resolver_preparse+0xc9f/0xd6=
0 net/dns_resolver/dns_key.c:127
        Read of size 1 at addr ffff888028894084 by task syz-executor265/50=
69
        ...
        Call Trace:
         <TASK>
         __dump_stack lib/dump_stack.c:88 [inline]
         dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
         print_address_description mm/kasan/report.c:377 [inline]
         print_report+0xc3/0x620 mm/kasan/report.c:488
         kasan_report+0xd9/0x110 mm/kasan/report.c:601
         dns_resolver_preparse+0xc9f/0xd60 net/dns_resolver/dns_key.c:127
         __key_create_or_update+0x453/0xdf0 security/keys/key.c:842
         key_create_or_update+0x42/0x50 security/keys/key.c:1007
         __do_sys_add_key+0x29c/0x450 security/keys/keyctl.c:134
         do_syscall_x64 arch/x86/entry/common.c:52 [inline]
         do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
         entry_SYSCALL_64_after_hwframe+0x62/0x6a

This patch was originally by Edward Adam Davis, but was modified by Linus.

Fixes: b946001d3bb1 ("keys, dns: Allow key types (eg. DNS) to be reclaimed=
 immediately on expiry")
Reported-and-tested-by: syzbot+94bbb75204a05da3d89f@syzkaller.appspotmail.=
com
Link: https://lore.kernel.org/r/0000000000009b39bc060c73e209@google.com/
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: David Howells <dhowells@redhat.com>
cc: Edward Adam Davis <eadavis@qq.com>
cc: Simon Horman <horms@kernel.org>
cc: Linus Torvalds <torvalds@linux-foundation.org>
cc: Jarkko Sakkinen <jarkko@kernel.org>
cc: Jeffrey E Altman <jaltman@auristor.com>
cc: Wang Lei <wang840925@gmail.com>
cc: Jeff Layton <jlayton@redhat.com>
cc: Steve French <sfrench@us.ibm.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: linux-cifs@vger.kernel.org
cc: linux-nfs@vger.kernel.org
cc: ceph-devel@vger.kernel.org
cc: keyrings@vger.kernel.org
cc: netdev@vger.kernel.org
---
 net/dns_resolver/dns_key.c |   19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 2a6d363763a2..f18ca02aa95a 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -91,8 +91,6 @@ const struct cred *dns_resolver_cache;
 static int
 dns_resolver_preparse(struct key_preparsed_payload *prep)
 {
-	const struct dns_server_list_v1_header *v1;
-	const struct dns_payload_header *bin;
 	struct user_key_payload *upayload;
 	unsigned long derrno;
 	int ret;
@@ -103,27 +101,28 @@ dns_resolver_preparse(struct key_preparsed_payload *=
prep)
 		return -EINVAL;
 =

 	if (data[0] =3D=3D 0) {
+		const struct dns_server_list_v1_header *v1;
+
 		/* It may be a server list. */
-		if (datalen <=3D sizeof(*bin))
+		if (datalen <=3D sizeof(*v1))
 			return -EINVAL;
 =

-		bin =3D (const struct dns_payload_header *)data;
-		kenter("[%u,%u],%u", bin->content, bin->version, datalen);
-		if (bin->content !=3D DNS_PAYLOAD_IS_SERVER_LIST) {
+		v1 =3D (const struct dns_server_list_v1_header *)data;
+		kenter("[%u,%u],%u", v1->hdr.content, v1->hdr.version, datalen);
+		if (v1->hdr.content !=3D DNS_PAYLOAD_IS_SERVER_LIST) {
 			pr_warn_ratelimited(
 				"dns_resolver: Unsupported content type (%u)\n",
-				bin->content);
+				v1->hdr.content);
 			return -EINVAL;
 		}
 =

-		if (bin->version !=3D 1) {
+		if (v1->hdr.version !=3D 1) {
 			pr_warn_ratelimited(
 				"dns_resolver: Unsupported server list version (%u)\n",
-				bin->version);
+				v1->hdr.version);
 			return -EINVAL;
 		}
 =

-		v1 =3D (const struct dns_server_list_v1_header *)bin;
 		if ((v1->status !=3D DNS_LOOKUP_GOOD &&
 		     v1->status !=3D DNS_LOOKUP_GOOD_WITH_BAD)) {
 			if (prep->expiry =3D=3D TIME64_MAX)