Bug 199705 - [patch] [geom] use-after-free bug in geli
Summary: [patch] [geom] use-after-free bug in geli
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Pawel Jakub Dawidek
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-04-26 11:07 UTC by luke.tw
Modified: 2016-01-31 14:41 UTC (History)
3 users (show)

See Also:


Attachments
patch for geli (2.06 KB, patch)
2015-04-26 11:07 UTC, luke.tw
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description luke.tw 2015-04-26 11:07:14 UTC
Created attachment 156004 [details]
patch for geli

In g_eli_auth_run() and g_eli_crypto_run(), crypto_dispatch() sends crypto request. 
After the last child bio is served, the bp is freed in g_vfs_done(). 
Then in g_eli_auth_run() and g_eli_crypto_run(), there are uses of the freed bp 

if (bp->bio_error == 0)
          bp->bio_error = error;
Comment 1 Ed Maste freebsd_committer 2015-07-19 18:19:40 UTC
Testing now.

I believe this will in practice be rare - let me know if you agree:

> crypto_dispatch()
> returns EINVAL if its argument or the callback function was NULL, and 0
> otherwise.

So aside from bugs elsewhere, crypto_dispatch should only ever return 0, and we'd need a race with another thread that sets bp->bio_error in order to cause an issue.
Comment 2 luke.tw 2015-07-19 23:38:50 UTC
(In reply to Ed Maste from comment #1)

Yes. I do believe this is a rare case in practice. :)
I run into this because I test tools/regression/geom_eli with memguard.
Comment 3 commit-hook freebsd_committer 2015-08-06 17:14:02 UTC
A commit references this bug:

Author: pjd
Date: Thu Aug  6 17:13:35 UTC 2015
New revision: 286373
URL: https://svnweb.freebsd.org/changeset/base/286373

Log:
  After crypto_dispatch() bio might be already delivered and destroyed,
  so we cannot access it anymore. Setting an error later lead to memory
  corruption.

  Assert that crypto_dispatch() was successful. It can fail only if we pass a
  bogus crypto request, which is a bug in the program, not a runtime condition.

  PR:		199705
  Submitted by:	luke.tw
  Reviewed by:	emaste
  MFC after:	3 days

Changes:
  head/sys/geom/eli/g_eli_integrity.c
  head/sys/geom/eli/g_eli_privacy.c