Paths

Table of Contentst

Differential D60162

epoch: Fix use-after-free in epoch_trace_report()
ClosedPublic
Actions

Authored by rcm on Wed, Sep 30, 2:21 AM.

Tags

None

Referenced Files

	F174104865: D60162.diff
	Wed, Sep 30, 4:11 PM

	F174090160: D60162.diff
	Wed, Sep 30, 1:32 PM

	F174090137: D60162.diff
	Wed, Sep 30, 1:32 PM

	F174089949: D60162.diff
	Wed, Sep 30, 1:29 PM

Subscribers

Details

Reviewers

markj
glebius

Commits

rGdd5dc8f6e51c: epoch: Fix use-after-free in epoch_trace_report()

Summary

epoch_trace_report() assigned the return value of RB_INSERT() back to
the new element. When two threads report the same stack concurrently,
the loser's RB_INSERT() returns the element already in the tree, and
that element was freed while still linked, leaking the new allocation.
The next lookup touches freed memory; KASAN catches it as a
use-after-free.

Keep the return value separate and free the new element instead. The
thread that won the race prints the report, so return without printing
it a second time.

Fixes: 173c062a569b ("Improve EPOCH_TRACE")
MFC after: 1 week
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

rcm created this revision.Wed, Sep 30, 2:21 AM

Herald added subscribers: olce, imp. · View Herald TranscriptWed, Sep 30, 2:21 AM

rcm requested review of this revision.Wed, Sep 30, 2:21 AM

rcm added reviewers: markj, glebius.

markj accepted this revision.Wed, Sep 30, 1:30 PM

markj added inline comments.

sys/kern/subr_epoch.c
209	You could make `dup` a bool and set `dup = RB_INSERT(...) != NULL;`. That might be a bit clearer, just a suggestion.

This revision is now accepted and ready to land.Wed, Sep 30, 1:30 PM

Made dup a bool set from RB_INSERT(...) != NULL, as suggested. I agree; that reads better.

This revision now requires review to proceed.Wed, Sep 30, 1:40 PM

rcm marked an inline comment as done.Wed, Sep 30, 1:40 PM

markj accepted this revision.Wed, Sep 30, 1:41 PM

This revision is now accepted and ready to land.Wed, Sep 30, 1:41 PM

Closed by commit rGdd5dc8f6e51c: epoch: Fix use-after-free in epoch_trace_report() (authored by rcm). · Explain WhyWed, Sep 30, 1:51 PM

This revision was automatically updated to reflect the committed changes.

rcm added a commit: rGdd5dc8f6e51c: epoch: Fix use-after-free in epoch_trace_report().

Revision Contents
Changeset List

Path

Size

sys/

kern/

8 lines

Diff 188198

sys/kern/subr_epoch.c

Loading...