APPLIED[F]: [E/F/Unstable][PATCH 0/1] crypto: fix regression/use-after-free in af_alg_accept()

[Kelsey Skunberg]

Applied to Focal/master-next. Thank you!

-Kelsey

On 2020-06-30 20:09:41 , Mauricio Faria de Oliveira wrote:
> Regarding the submission policies for this kernel SRU cycle:
> this patch does not necessarily have to be applied for now;
> just review/ack for B/E would be useful if at all possible.
> 
> It has only been merged on Linus' tree yesterday.
> 
> The patch applies cleanly on Unstable/Focal/Eoan,
> and is a trivial backport on Disco/Bionic/Xenial.
> (it's on all series because it's a fix to stable.)
> 
> [Impact]
> 
>  * Users of the Linux kernel's crypto userspace API
>    reported BUG() / kernel NULL pointer dereference
>    errors after kernel upgrades.
> 
>  * The stack trace signature is an accept() syscall
>    going through af_alg_accept() and hitting errors
>    usually in one of:
>    - apparmor_sk_clone_security()
>    - apparmor_sock_graft()
>    - release_sock()
> 
> [Fix]
>       
>  * This is a regression introduced by upstream commit
>    37f96694cf73 ("crypto: af_alg - Use bh_lock_sock
>    in sk_destruct") which made its way through stable.
>    
>  * The offending patch allows the critical regions
>    of af_alg_accept() and af_alg_release_parent() to
>    run concurrently; now with the "right" events on 2
>    CPUs it might drop the non-atomic reference counter
>    of the alg_sock then the sock, thus release a sock
>    that is still in use.
> 
>  * The fix is upstream commit 34c86f4c4a7b ("crypto:
>    af_alg - fix use-after-free in af_alg_accept() due
>    to bh_lock_sock()") [1]. It changes alg_sock's ref
>    counter to atomic, which addresses the root cause.
>    
> [Test Case]
> 
>  * There is a synthetic test case available, which
>    uses a kprobes kernel module to synchronize the
>    concurrent CPUs on the instructions responsible
>    for the problem; and a userspace part to run it.
> 
>  * The organic reproducer is the Varnish Cache Plus
>    software with the Crypto vmod (which uses kernel
>    crypto userspace API) under long, very high load.
>    
>  * The patch has been verified on both reproducers
>    with the 4.15 and 5.7 kernels.
>    
>  * More tests performed with 'stress-ng --af-alg'
>    with 11 CPUs on Xenial/Bionic/Disco/Eoan/Focal
>    (all on same version of stress-ng, V0.11.14)
>    
>    No regressions observed from original kernel.
>    (the af-alg stressor can exercise almost all
>    kernel crypto modules shipped with the kernel;
>    so it checks more paths/crypto alg interfaces.)
>    
> [Regression Potential]
> 
>  * The fix patch does a fundamental change in how
>    alg_sock reference counters work, plus another
>    change to the 'nokey' counting. This of course
>    *has* a risk of regression.
> 
>  * Regressions theoretically could manifest as use
>    after free errors (in case of undercounting) in
>    the af_alg functions or silent memory leaks (in
>    case of overcounting), but also other behaviors
>    since reference counting is key to many things.
>    
>  * FWIW, this patch has been written by the crypto
>    subsystem maintainer, who certainly knows a lot
>    of the normal and corner cases, thus giving the
>    patch more credit.
>    
>  * Testing with the organic reproducer ran as long
>    as 5 days, without issues, so it does look good.
> 
> [Other Info]
> 
>  * Not sending for Groovy (should get via Unstable).
>  
>  * [1] Patch: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34c86f4c4a7be3b3e35aa48bd18299d4c756064d
>  
> [Stack Trace Examples]
> 
> Examples:
> 
>     BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
>     ...
>     RIP: 0010:apparmor_sk_clone_security+0x26/0x70
>     ...
>     Call Trace:
>      security_sk_clone+0x33/0x50
>      af_alg_accept+0x81/0x1c0 [af_alg]
>      alg_accept+0x15/0x20 [af_alg]
>      SYSC_accept4+0xff/0x210
>      SyS_accept+0x10/0x20
>      do_syscall_64+0x73/0x130
>      entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> 
>     general protection fault: 0000 [#1] SMP PTI
>     ...
>     RIP: 0010:__release_sock+0x54/0xe0
>     ...
>     Call Trace:
>      release_sock+0x30/0xa0
>      af_alg_accept+0x122/0x1c0 [af_alg]
>      alg_accept+0x15/0x20 [af_alg]
>      SYSC_accept4+0xff/0x210
>      SyS_accept+0x10/0x20
>      do_syscall_64+0x73/0x130
>      entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> 
> Herbert Xu (1):
>   crypto: af_alg - fix use-after-free in af_alg_accept() due to
>     bh_lock_sock()
> 
>  crypto/af_alg.c         | 26 +++++++++++---------------
>  crypto/algif_aead.c     |  9 +++------
>  crypto/algif_hash.c     |  9 +++------
>  crypto/algif_skcipher.c |  9 +++------
>  include/crypto/if_alg.h |  4 ++--
>  5 files changed, 22 insertions(+), 35 deletions(-)
> 
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team