Re: [syzbot] [ext4?] general protection fault in ext4_put_io_end_defer

Re: [syzbot] [ext4?] general protection fault in ext4_put_io_end_defer

[Theodore Ts'o]

#syz set subsystems: crypto

On Sat, Jun 24, 2023 at 07:21:44PM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    f7efed9f38f8 Add linux-next specific files for 20230616
> git tree:       linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=152e89f3280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=60b1a32485a77c16
> dashboard link: https://syzkaller.appspot.com/bug?extid=94a8c779c6b238870393
> compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=116af1eb280000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14e22d2f280000

If you look at the reproducer, it's creating an AF_ALG (algorithm)
socket and messing with it.  This is easier to see in the syz
reproducer, but you can see exactly what it's doing in the C
reproducer above:

# https://syzkaller.appspot.com/bug?id=4ee7656695de92cbd5820111379ae0698af0f475
# See https://goo.gl/kgGztJ for information about syzkaller reproducers.
#{"threaded":true,"repeat":true,"procs":1,"slowdown":1,"sandbox":"none","sandbox_arg":0,"netdev":true,"binfmt_misc":true,"close_fds":true,"vhci":true,"ieee802154":true,"sysctl":true,"swap":true,"tmpdir":true}
r0 = socket$alg(0x26, 0x5, 0x0)
bind$alg(r0, &(0x7f0000000280)={0x26, 'hash\x00', 0x0, 0x0, 'sha3-256-generic\x00'}, 0x58)
r1 = accept4(r0, 0x0, 0x0, 0x0)
recvmmsg$unix(r1, &(0x7f0000003700)=[{{0x0, 0x700, 0x0}}], 0x600, 0x0, 0x0)
sendmsg$can_bcm(r1, &(0x7f0000000180)={0x0, 0x0, &(0x7f0000000140)={0x0}}, 0x400c800)

(0x26 is 38, or AF_ALG)

From looking at the stack trace, it looks like this is triggering a
coredump, which presumably is the ext4 write that triggers the GPF in
ext4_put_io_end_defer.  But given that the syz and C reproducer isn't
doing anything ext4 related at all, and it's purely trying to use the
AF_ALG socket to calculate SHA3 in the kernel (and the greek chorus
cries out, "WHY?"[1]), I'm going to send this over to the crypto folks to
investigate.

Cheers,

					- Ted

[1] TIL that AF_ALG exists.  Inquiring minds want to know:
   * Why do we expose the AF_ALG userspace interface?
   * Who uses it?
   * Why do they use it?
   * Is there a CONFIG option to disable it in the name of decreasing
     the attack surface of the kernel?
   * If not, should we add one?  :-)

Re: [syzbot] [ext4?] general protection fault in ext4_put_io_end_defer

[Eric Biggers]

On Wed, Jun 28, 2023 at 11:57:14PM -0400, Theodore Ts'o wrote:
> #syz set subsystems: crypto
> 
> On Sat, Jun 24, 2023 at 07:21:44PM -0700, syzbot wrote:
> > Hello,
> > 
> > syzbot found the following issue on:
> > 
> > HEAD commit:    f7efed9f38f8 Add linux-next specific files for 20230616
> > git tree:       linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=152e89f3280000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=60b1a32485a77c16
> > dashboard link: https://syzkaller.appspot.com/bug?extid=94a8c779c6b238870393
> > compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=116af1eb280000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14e22d2f280000
> 
> If you look at the reproducer, it's creating an AF_ALG (algorithm)
> socket and messing with it.  This is easier to see in the syz
> reproducer, but you can see exactly what it's doing in the C
> reproducer above:
> 
> # https://syzkaller.appspot.com/bug?id=4ee7656695de92cbd5820111379ae0698af0f475
> # See https://goo.gl/kgGztJ for information about syzkaller reproducers.
> #{"threaded":true,"repeat":true,"procs":1,"slowdown":1,"sandbox":"none","sandbox_arg":0,"netdev":true,"binfmt_misc":true,"close_fds":true,"vhci":true,"ieee802154":true,"sysctl":true,"swap":true,"tmpdir":true}
> r0 = socket$alg(0x26, 0x5, 0x0)
> bind$alg(r0, &(0x7f0000000280)={0x26, 'hash\x00', 0x0, 0x0, 'sha3-256-generic\x00'}, 0x58)
> r1 = accept4(r0, 0x0, 0x0, 0x0)
> recvmmsg$unix(r1, &(0x7f0000003700)=[{{0x0, 0x700, 0x0}}], 0x600, 0x0, 0x0)
> sendmsg$can_bcm(r1, &(0x7f0000000180)={0x0, 0x0, &(0x7f0000000140)={0x0}}, 0x400c800)
> 
> (0x26 is 38, or AF_ALG)
> 
> From looking at the stack trace, it looks like this is triggering a
> coredump, which presumably is the ext4 write that triggers the GPF in
> ext4_put_io_end_defer.  But given that the syz and C reproducer isn't
> doing anything ext4 related at all, and it's purely trying to use the
> AF_ALG socket to calculate SHA3 in the kernel (and the greek chorus
> cries out, "WHY?"[1]), I'm going to send this over to the crypto folks to
> investigate.

Just a couple weeks ago, commit c662b043cdca ("crypto: af_alg/hash: Support
MSG_SPLICE_PAGES") had many syzbot reports against it.  This particular report
is against next-20230616 which didn't include the fix commit b6d972f68983
("crypto: af_alg/hash: Fix recvmsg() after sendmsg(MSG_MORE)").  So there's a
high chance this report is no longer valid.  I'll go ahead and invalidate it:

#syz invalid

> 
> Cheers,
> 
> 					- Ted
> 
> [1] TIL that AF_ALG exists.  Inquiring minds want to know:
>    * Why do we expose the AF_ALG userspace interface?
>    * Who uses it?
>    * Why do they use it?
>    * Is there a CONFIG option to disable it in the name of decreasing
>      the attack surface of the kernel?
>    * If not, should we add one?  :-)

AF_ALG has existed since 2010.  My understanding that its original purpose was
to expose hardware crypto accelerators to userspace.  Unfortunately, support for
exposing *any* crypto algorithm was included as well, which IMO was a mistake.

There are quite a few different userspace programs that use AF_ALG purely to get
at the CPU-based algorithm implementations, without any sort of intention to use
hardware crypto accelerator.  Probably because it seemed "easy".  Or "better"
because everything in the kernel is better, right?

It's controlled by the CONFIG_CRYPTO_USER_API_* options, with the hash support
in particular controlled by CONFIG_CRYPTO_USER_API_HASH.  Though good luck
disabling it on most systems, as systemd depends on it...

- Eric

Re: [syzbot] [ext4?] general protection fault in ext4_put_io_end_defer

[Eric Biggers]

On Fri, Jun 30, 2023 at 12:41:11AM -0700, Eric Biggers wrote:
> On Wed, Jun 28, 2023 at 11:57:14PM -0400, Theodore Ts'o wrote:
> > #syz set subsystems: crypto
> > 
> > On Sat, Jun 24, 2023 at 07:21:44PM -0700, syzbot wrote:
> > > Hello,
> > > 
> > > syzbot found the following issue on:
> > > 
> > > HEAD commit:    f7efed9f38f8 Add linux-next specific files for 20230616
> > > git tree:       linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=152e89f3280000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=60b1a32485a77c16
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=94a8c779c6b238870393
> > > compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=116af1eb280000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14e22d2f280000
> > 
> > If you look at the reproducer, it's creating an AF_ALG (algorithm)
> > socket and messing with it.  This is easier to see in the syz
> > reproducer, but you can see exactly what it's doing in the C
> > reproducer above:
> > 
> > # https://syzkaller.appspot.com/bug?id=4ee7656695de92cbd5820111379ae0698af0f475
> > # See https://goo.gl/kgGztJ for information about syzkaller reproducers.
> > #{"threaded":true,"repeat":true,"procs":1,"slowdown":1,"sandbox":"none","sandbox_arg":0,"netdev":true,"binfmt_misc":true,"close_fds":true,"vhci":true,"ieee802154":true,"sysctl":true,"swap":true,"tmpdir":true}
> > r0 = socket$alg(0x26, 0x5, 0x0)
> > bind$alg(r0, &(0x7f0000000280)={0x26, 'hash\x00', 0x0, 0x0, 'sha3-256-generic\x00'}, 0x58)
> > r1 = accept4(r0, 0x0, 0x0, 0x0)
> > recvmmsg$unix(r1, &(0x7f0000003700)=[{{0x0, 0x700, 0x0}}], 0x600, 0x0, 0x0)
> > sendmsg$can_bcm(r1, &(0x7f0000000180)={0x0, 0x0, &(0x7f0000000140)={0x0}}, 0x400c800)
> > 
> > (0x26 is 38, or AF_ALG)
> > 
> > From looking at the stack trace, it looks like this is triggering a
> > coredump, which presumably is the ext4 write that triggers the GPF in
> > ext4_put_io_end_defer.  But given that the syz and C reproducer isn't
> > doing anything ext4 related at all, and it's purely trying to use the
> > AF_ALG socket to calculate SHA3 in the kernel (and the greek chorus
> > cries out, "WHY?"[1]), I'm going to send this over to the crypto folks to
> > investigate.
> 
> Just a couple weeks ago, commit c662b043cdca ("crypto: af_alg/hash: Support
> MSG_SPLICE_PAGES") had many syzbot reports against it.  This particular report
> is against next-20230616 which didn't include the fix commit b6d972f68983
> ("crypto: af_alg/hash: Fix recvmsg() after sendmsg(MSG_MORE)").  So there's a
> high chance this report is no longer valid.  I'll go ahead and invalidate it:
> 
> #syz invalid
> 
> > 
> > Cheers,
> > 
> > 					- Ted
> > 
> > [1] TIL that AF_ALG exists.  Inquiring minds want to know:
> >    * Why do we expose the AF_ALG userspace interface?
> >    * Who uses it?
> >    * Why do they use it?
> >    * Is there a CONFIG option to disable it in the name of decreasing
> >      the attack surface of the kernel?
> >    * If not, should we add one?  :-)
> 
> AF_ALG has existed since 2010.  My understanding that its original purpose was
> to expose hardware crypto accelerators to userspace.  Unfortunately, support for
> exposing *any* crypto algorithm was included as well, which IMO was a mistake.
> 
> There are quite a few different userspace programs that use AF_ALG purely to get
> at the CPU-based algorithm implementations, without any sort of intention to use
> hardware crypto accelerator.  Probably because it seemed "easy".  Or "better"
> because everything in the kernel is better, right?
> 
> It's controlled by the CONFIG_CRYPTO_USER_API_* options, with the hash support
> in particular controlled by CONFIG_CRYPTO_USER_API_HASH.  Though good luck
> disabling it on most systems, as systemd depends on it...
> 

Actually it turns out systemd has finally seen the light:
https://github.com/systemd/systemd/commit/2c3794f4228162c9bfd9e10886590d9f5b1920d7

- Eric

Re: [syzbot] [ext4?] general protection fault in ext4_put_io_end_defer

[Theodore Ts'o]

On Fri, Jun 30, 2023 at 12:46:14AM -0700, Eric Biggers wrote:
> > AF_ALG has existed since 2010.  My understanding that its original purpose was
> > to expose hardware crypto accelerators to userspace.  Unfortunately, support for
> > exposing *any* crypto algorithm was included as well, which IMO was a mistake.

+1000....

> > There are quite a few different userspace programs that use AF_ALG purely to get
> > at the CPU-based algorithm implementations, without any sort of intention to use
> > hardware crypto accelerator.  Probably because it seemed "easy".  Or "better"
> > because everything in the kernel is better, right?

Do we know if any to standard crypto libraries are using AF_ALG?  All
aside from whether it's a good idea for userspace programs to be using
kernel code because "everything is better in the kernel", I'm
wondering how solicitous we should be for programs who are very likely
rolling their own crypto, as opposed to using crypto library that has
been written and vetted and tested for vulnerability by experts...

> > It's controlled by the CONFIG_CRYPTO_USER_API_* options, with the hash support
> > in particular controlled by CONFIG_CRYPTO_USER_API_HASH.  Though good luck
> > disabling it on most systems, as systemd depends on it...
> > 
> 
> Actually it turns out systemd has finally seen the light:
> https://github.com/systemd/systemd/commit/2c3794f4228162c9bfd9e10886590d9f5b1920d7

Aside from those PCI-attached crypto accelerators where you have to go
through the kernel (although my experience has been that most of the
time, the overhead for key scheduling, etc., is such that unless
you're doing bulk crypto on large chunks of data, using external
crypto hardware acclerators no longer makes sense 99.99% of the time
in the 21st century), I wonder if we should consider having the kernel
print a warning, "WARNING: [comm] is using AF_ALG; please consider
using a real crypto library instead of rolling your own crypto".

(Only half kidding.)

					- Ted