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
| ||
|
Message-ID: <20201106070122.GC11620@gondor.apana.org.au> Date: Fri, 6 Nov 2020 18:01:22 +1100 From: Herbert Xu <herbert@...dor.apana.org.au> To: Eric Biggers <ebiggers@...nel.org> Cc: linux-crypto@...r.kernel.org, syzkaller-bugs@...glegroups.com, linux-hardening@...r.kernel.org, linux-api@...r.kernel.org, linux-kernel@...r.kernel.org, Jann Horn <jannh@...gle.com>, Kees Cook <keescook@...omium.org>, Elena Petrova <lenaptr@...gle.com>, Vegard Nossum <vegard.nossum@...cle.com>, "Gustavo A . R . Silva" <gustavoars@...nel.org>, stable@...r.kernel.org, syzbot+92ead4eb8e26a26d465e@...kaller.appspotmail.com Subject: Re: [PATCH] crypto: af_alg - avoid undefined behavior accessing salg_name On Mon, Oct 26, 2020 at 01:07:15PM -0700, Eric Biggers wrote: > From: Eric Biggers <ebiggers@...gle.com> > > Commit 3f69cc60768b ("crypto: af_alg - Allow arbitrarily long algorithm > names") made the kernel start accepting arbitrarily long algorithm names > in sockaddr_alg. However, the actual length of the salg_name field > stayed at the original 64 bytes. > > This is broken because the kernel can access indices >= 64 in salg_name, > which is undefined behavior -- even though the memory that is accessed > is still located within the sockaddr structure. It would only be > defined behavior if the array were properly marked as arbitrary-length > (either by making it a flexible array, which is the recommended way > these days, or by making it an array of length 0 or 1). > > We can't simply change salg_name into a flexible array, since that would > break source compatibility with userspace programs that embed > sockaddr_alg into another struct, or (more commonly) declare a > sockaddr_alg like 'struct sockaddr_alg sa = { .salg_name = "foo" };'. > > One solution would be to change salg_name into a flexible array only > when '#ifdef __KERNEL__'. However, that would keep userspace without an > easy way to actually use the longer algorithm names. > > Instead, add a new structure 'sockaddr_alg_new' that has the flexible > array field, and expose it to both userspace and the kernel. > Make the kernel use it correctly in alg_bind(). > > This addresses the syzbot report > "UBSAN: array-index-out-of-bounds in alg_bind" > (https://syzkaller.appspot.com/bug?extid=92ead4eb8e26a26d465e). > > Reported-by: syzbot+92ead4eb8e26a26d465e@...kaller.appspotmail.com > Fixes: 3f69cc60768b ("crypto: af_alg - Allow arbitrarily long algorithm names") > Cc: <stable@...r.kernel.org> # v4.12+ > Signed-off-by: Eric Biggers <ebiggers@...gle.com> > --- > crypto/af_alg.c | 10 +++++++--- > include/uapi/linux/if_alg.h | 16 ++++++++++++++++ > 2 files changed, 23 insertions(+), 3 deletions(-) Patch applied. Thanks. -- Email: Herbert Xu <herbert@...dor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Powered by blists - more mailing lists