| Summary: | [patch] [geom] use-after-free bug in geli | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | luke.tw | ||||
| Component: | kern | Assignee: | Pawel Jakub Dawidek <pjd> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | CC: | allanjude, emaste, pjd | ||||
| Priority: | --- | Keywords: | patch | ||||
| Version: | CURRENT | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
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.
(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. 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 |
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;