Bug 230304

Summary: Malloc while lock is held in crypto code
Product: Base System Reporter: Sean Eric Fagan <sef>
Component: kernAssignee: Conrad Meyer <cem>
Status: Closed FIXED    
Severity: Affects Some People    
Priority: ---    
Version: CURRENT   
Hardware: Any   
OS: Any   

Description Sean Eric Fagan freebsd_committer freebsd_triage 2018-08-02 20:17:05 UTC 
When using swcr_newsession(), changes in r336439 result in:

uma_zalloc_arg: zone "64" with the following non-sleepable locks held: exclusive sleep mutex crypto (crypto driver table) r = 0 (0xffffffff81faf330) locked @ /FreeBSD/src-git-new/sys/opencrypto/crypto.c:573 stack backtrace:
#0 0xffffffff80c00013 at witness_debugger+0x73
#1 0xffffffff80c013f1 at witness_warn+0x461
#2 0xffffffff80ed6478 at uma_zalloc_arg+0x38
#3 0xffffffff80b6d165 at malloc+0x95
#4 0xffffffff80e4ad35 at swcr_newsession+0x75
#5 0xffffffff80e47bb1 at crypto_newsession+0x3b1

Discussed with cem, filing a PR for tracking.
Comment 1 Conrad Meyer freebsd_committer freebsd_triage 2018-08-02 20:50:16 UTC 
The issue is that crypto_newsession() holds the normal mutex ("Crypto
driver table" / CRYPTO_DRIVER_LOCK()) over the CRYPTODEV_NEWSESSION()
call, and the cryptosoft driver allocates extra memory with M_WAITOK.

The short term solution is to change crypto driver allocations back to
M_NOWAIT unconditionally and be sure to check all error paths for bugs
and memory leaks.

But I don't think that's a reasonable long term approach.  I think we
probably want to pass a flags argument from crypto_newsession all the
way down into individual driver newsessions to fix this right.  That
will take a decent amount of churn.  crypto_newsession() also needs to
be fixed to avoid deadlock when the caller allows M_WAITOK by dropping
the lock over the call.
Comment 2 commit-hook freebsd_committer freebsd_triage 2018-08-17 04:40:26 UTC 
A commit references this bug:

Author: cem
Date: Fri Aug 17 04:40:01 UTC 2018
New revision: 337958
URL: https://svnweb.freebsd.org/changeset/base/337958

Log:
  cryptosoft: Reduce generality of supported algorithm composition

  Fix a regression introduced in r336439.

  Rather than allowing any linked list of algorithms, allow at most two
  (typically, some combination of encrypt and/or MAC).  Removes a WAITOK
  malloc in an unsleepable context (classic LOR) by placing both software
  algorithm contexts within the OCF-managed session object.

  Tested with 'cryptocheck -a all -d cryptosoft0', which includes some
  encrypt-and-MAC modes.

  PR:		230304
  Reported by:	sef@

Changes:
  head/sys/opencrypto/cryptosoft.c
  head/sys/opencrypto/cryptosoft.h