linux-kernel - Re: [PATCH] keys, dns: Fix missing size check of V1 server-list header

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231224102219.GB215659@kernel.org>
Date: Sun, 24 Dec 2023 10:22:19 +0000
From: Simon Horman <horms@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
	Edward Adam Davis <eadavis@...com>,
	Markus Suvanto <markus.suvanto@...il.com>,
	Jeffrey E Altman <jaltman@...istor.com>,
	Marc Dionne <marc.dionne@...istor.com>,
	Wang Lei <wang840925@...il.com>, Jeff Layton <jlayton@...hat.com>,
	Steve French <smfrench@...il.com>,
	Jarkko Sakkinen <jarkko@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	linux-afs@...ts.infradead.org, keyrings@...r.kernel.org,
	linux-cifs@...r.kernel.org, linux-nfs@...r.kernel.org,
	ceph-devel@...r.kernel.org, netdev@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] keys, dns: Fix missing size check of V1 server-list
 header

On Sun, Dec 24, 2023 at 12:02:49AM +0000, David Howells wrote:
> 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 made
> modifications or whether I should use a Codeveloped-by line for him.
> 
> David
> ---
> From: Edward Adam Davis <eadavis@...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 payload
> 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/0xd60 net/dns_resolver/dns_key.c:127
>         Read of size 1 at addr ffff888028894084 by task syz-executor265/5069
>         ...
>         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@...kaller.appspotmail.com
> Link: https://lore.kernel.org/r/0000000000009b39bc060c73e209@google.com/
> Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> Signed-off-by: David Howells <dhowells@...hat.com>
> Tested-by: David Howells <dhowells@...hat.com>

Thanks.

FWIIW, I prefer this approach where v1 and bin don't alias each other,
and the scope of v1 is constrained to the block where it is used.

Reviewed-by: Simon Horman <horms@...nel.org>

...