Bug 230304 - Malloc while lock is held in crypto code
Summary: Malloc while lock is held in crypto code
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Conrad Meyer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-02 20:17 UTC by Sean Eric Fagan
Modified: 2018-08-17 04:40 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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