Skip to content

Commit d44ca19

Browse files
ericnieblerericniebler
committed
fix use-after-free bug
1 parent 03ea969 commit d44ca19

3 files changed

Lines changed: 118 additions & 71 deletions

File tree

‎include/stdexec/__detail/__optional.hpp‎

Lines changed: 56 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
// include these after __execution_fwd.hpp
2121
#include "__concepts.hpp"
22+
#include "__scope.hpp"
2223

2324
#include <new> // IWYU pragma: keep for ::new
2425
#include <exception>
@@ -34,6 +35,13 @@ namespace stdexec {
3435
}
3536
};
3637

38+
inline auto __mk_has_value_guard(bool& __has_value) noexcept {
39+
__has_value = true;
40+
return __scope_guard{[&]() noexcept {
41+
__has_value = false;
42+
}};
43+
}
44+
3745
inline constexpr struct __nullopt_t {
3846
} __nullopt{};
3947

@@ -43,10 +51,10 @@ namespace stdexec {
4351
static_assert(destructible<_Tp>);
4452

4553
union {
46-
_Tp __value;
54+
_Tp __value_;
4755
};
4856

49-
bool __has_value = false;
57+
bool __has_value_ = false;
5058

5159
__optional() noexcept {
5260
}
@@ -58,88 +66,102 @@ namespace stdexec {
5866

5967
template <__not_decays_to<__optional> _Up>
6068
requires constructible_from<_Tp, _Up>
61-
__optional(_Up&& __v)
62-
: __value(static_cast<_Up&&>(__v))
63-
, __has_value(true) {
69+
__optional(_Up&& __v) noexcept(__nothrow_constructible_from<_Tp, _Up>) {
70+
emplace(static_cast<_Up&&>(__v));
6471
}
6572

6673
template <class... _Us>
6774
requires constructible_from<_Tp, _Us...>
68-
__optional(std::in_place_t, _Us&&... __us)
69-
: __value(static_cast<_Us&&>(__us)...)
70-
, __has_value(true) {
75+
__optional(std::in_place_t, _Us&&... __us) noexcept(
76+
__nothrow_constructible_from<_Tp, _Us...>) {
77+
emplace(static_cast<_Us&&>(__us)...);
7178
}
7279

7380
~__optional() {
74-
if (__has_value) {
75-
std::destroy_at(std::addressof(__value));
81+
if (__has_value_) {
82+
std::destroy_at(std::addressof(__value_));
7683
}
7784
}
7885

86+
// The following emplace function must take great care to avoid use-after-free bugs.
87+
// If the object being constructed calls `start` on a newly created operation state
88+
// (as does the object returned from `submit`), and if `start` completes inline, it
89+
// could cause the destruction of the outer operation state that owns *this. The
90+
// function below uses the following pattern to avoid this:
91+
// 1. Set __has_value_ to true.
92+
// 2. Create a scope guard that will reset __has_value_ to false if the constructor
93+
// throws.
94+
// 3. Construct the new object in the storage, which may cause the invalidation of
95+
// *this. The emplace function must not access any members of *this after this point.
96+
// 4. Dismiss the scope guard, which will leave __has_value_ set to true.
97+
// 5. Return a reference to the new object -- which may be invalid! Calling code
98+
// must be aware of the danger.
7999
template <class... _Us>
80100
requires constructible_from<_Tp, _Us...>
81101
auto emplace(_Us&&... __us) noexcept(__nothrow_constructible_from<_Tp, _Us...>) -> _Tp& {
82-
reset(); // sets __has_value to false in case the next line throws
83-
::new (&__value) _Tp{static_cast<_Us&&>(__us)...};
84-
__has_value = true;
85-
return __value;
102+
reset();
103+
auto __sg = __mk_has_value_guard(__has_value_);
104+
auto* __p = ::new (static_cast<void*>(std::addressof(__value_)))
105+
_Tp{static_cast<_Us&&>(__us)...};
106+
__sg.__dismiss();
107+
return *std::launder(__p);
86108
}
87109

88110
auto value() & -> _Tp& {
89-
if (!__has_value) {
111+
if (!__has_value_) {
90112
throw __bad_optional_access();
91113
}
92-
return __value;
114+
return __value_;
93115
}
94116

95117
auto value() const & -> const _Tp& {
96-
if (!__has_value) {
118+
if (!__has_value_) {
97119
throw __bad_optional_access();
98120
}
99-
return __value;
121+
return __value_;
100122
}
101123

102124
auto value() && -> _Tp&& {
103-
if (!__has_value) {
125+
if (!__has_value_) {
104126
throw __bad_optional_access();
105127
}
106-
return static_cast<_Tp&&>(__value);
128+
return static_cast<_Tp&&>(__value_);
107129
}
108130

109131
auto operator*() & noexcept -> _Tp& {
110-
STDEXEC_ASSERT(__has_value);
111-
return __value;
132+
STDEXEC_ASSERT(__has_value_);
133+
return __value_;
112134
}
113135

114136
auto operator*() const & noexcept -> const _Tp& {
115-
STDEXEC_ASSERT(__has_value);
116-
return __value;
137+
STDEXEC_ASSERT(__has_value_);
138+
return __value_;
117139
}
118140

119141
auto operator*() && noexcept -> _Tp&& {
120-
STDEXEC_ASSERT(__has_value);
121-
return static_cast<_Tp&&>(__value);
142+
STDEXEC_ASSERT(__has_value_);
143+
return static_cast<_Tp&&>(__value_);
122144
}
123145

124146
auto operator->() & noexcept -> _Tp* {
125-
STDEXEC_ASSERT(__has_value);
126-
return &__value;
147+
STDEXEC_ASSERT(__has_value_);
148+
return &__value_;
127149
}
128150

129151
auto operator->() const & noexcept -> const _Tp* {
130-
STDEXEC_ASSERT(__has_value);
131-
return &__value;
152+
STDEXEC_ASSERT(__has_value_);
153+
return &__value_;
132154
}
133155

134156
[[nodiscard]]
135157
auto has_value() const noexcept -> bool {
136-
return __has_value;
158+
return __has_value_;
137159
}
138160

139161
void reset() noexcept {
140-
if (__has_value) {
141-
std::destroy_at(std::addressof(__value));
142-
__has_value = false;
162+
if (__has_value_) {
163+
std::destroy_at(std::addressof(__value_));
164+
__has_value_ = false;
143165
}
144166
}
145167
};

‎include/stdexec/__detail/__submit.hpp‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -70,35 +70,35 @@ namespace stdexec {
7070
static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr)};
7171
}
7272

73-
// This implementation is used if the sender has a non-static submit member function.
74-
template <class _Sender, class _Receiver, class _Default = __void>
75-
requires sender_to<_Sender, _Receiver> && __submit::__has_memfn<_Sender, _Receiver>
76-
auto operator()(_Sender&& __sndr, _Receiver __rcvr, [[maybe_unused]] _Default __def = {}) const
77-
noexcept(noexcept(static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr)))) {
78-
using __result_t =
79-
decltype(static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr)));
80-
if constexpr (__same_as<__result_t, void> && !__same_as<_Default, __void>) {
81-
static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr));
82-
return __def;
83-
} else {
84-
return static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr));
85-
}
86-
}
73+
// // This implementation is used if the sender has a non-static submit member function.
74+
// template <class _Sender, class _Receiver, class _Default = __void>
75+
// requires sender_to<_Sender, _Receiver> && __submit::__has_memfn<_Sender, _Receiver>
76+
// auto operator()(_Sender&& __sndr, _Receiver __rcvr, [[maybe_unused]] _Default __def = {}) const
77+
// noexcept(noexcept(static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr)))) {
78+
// using __result_t =
79+
// decltype(static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr)));
80+
// if constexpr (__same_as<__result_t, void> && !__same_as<_Default, __void>) {
81+
// static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr));
82+
// return __def;
83+
// } else {
84+
// return static_cast<_Sender&&>(__sndr).submit(static_cast<_Receiver&&>(__rcvr));
85+
// }
86+
// }
8787

88-
// This implementation is used if the sender has a static submit member function.
89-
template <class _Sender, class _Receiver, class _Default = __void>
90-
requires sender_to<_Sender, _Receiver> && __submit::__has_static_memfn<_Sender, _Receiver>
91-
auto operator()(_Sender&& __sndr, _Receiver __rcvr, [[maybe_unused]] _Default __def = {}) const
92-
noexcept(noexcept(__sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr)))) {
93-
using __result_t =
94-
decltype(__sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr)));
95-
if constexpr (__same_as<__result_t, void> && !__same_as<_Default, __void>) {
96-
__sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr));
97-
return __def;
98-
} else {
99-
return __sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr));
100-
}
101-
}
88+
// // This implementation is used if the sender has a static submit member function.
89+
// template <class _Sender, class _Receiver, class _Default = __void>
90+
// requires sender_to<_Sender, _Receiver> && __submit::__has_static_memfn<_Sender, _Receiver>
91+
// auto operator()(_Sender&& __sndr, _Receiver __rcvr, [[maybe_unused]] _Default __def = {}) const
92+
// noexcept(noexcept(__sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr)))) {
93+
// using __result_t =
94+
// decltype(__sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr)));
95+
// if constexpr (__same_as<__result_t, void> && !__same_as<_Default, __void>) {
96+
// __sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr));
97+
// return __def;
98+
// } else {
99+
// return __sndr.submit(static_cast<_Sender&&>(__sndr), static_cast<_Receiver&&>(__rcvr));
100+
// }
101+
// }
102102
};
103103

104104
inline constexpr __submit_t submit{};

‎include/stdexec/__detail/__variant.hpp‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "__meta.hpp"
1919
#include "__type_traits.hpp"
2020
#include "__utility.hpp"
21+
#include "__scope.hpp"
2122

2223
#include <cstddef>
2324
#include <memory>
@@ -40,6 +41,13 @@ namespace stdexec {
4041
struct __monostate { };
4142

4243
namespace __var {
44+
STDEXEC_ATTRIBUTE((host, device)) inline auto __mk_index_guard(std::size_t &__index, std::size_t __new) noexcept {
45+
__index = __new;
46+
return __scope_guard{[&__index]() noexcept {
47+
__index = __variant_npos;
48+
}};
49+
}
50+
4351
template <auto _Idx, class... _Ts>
4452
class __variant;
4553

@@ -102,16 +110,30 @@ namespace stdexec {
102110
return __index_ == __variant_npos;
103111
}
104112

113+
// The following emplace functions must take great care to avoid use-after-free bugs.
114+
// If the object being constructed calls `start` on a newly created operation state
115+
// (as does the object returned from `submit`), and if `start` completes inline, it
116+
// could cause the destruction of the outer operation state that owns *this. The
117+
// function below uses the following pattern to avoid this:
118+
// 1. Store the new index in __index_.
119+
// 2. Create a scope guard that will reset __index_ to __variant_npos if the
120+
// constructor throws.
121+
// 3. Construct the new object in the storage, which may cause the invalidation of
122+
// *this. The emplace function must not access any members of *this after this point.
123+
// 4. Dismiss the scope guard, which will leave __index_ set to the new index.
124+
// 5. Return a reference to the new object -- which may be invalid! Calling code
125+
// must be aware of the danger.
105126
template <class _Ty, class... _As>
106127
STDEXEC_ATTRIBUTE((host, device)) auto emplace(_As &&...__as) //
107128
noexcept(__nothrow_constructible_from<_Ty, _As...>) -> _Ty & {
108129
constexpr std::size_t __new_index = stdexec::__index_of<_Ty, _Ts...>();
109130
static_assert(__new_index != __variant_npos, "Type not in variant");
110131

111132
__destroy();
112-
::new (__storage_) _Ty{static_cast<_As &&>(__as)...};
113-
__index_ = __new_index;
114-
return *std::launder(reinterpret_cast<_Ty *>(__storage_));
133+
auto __sg = __mk_index_guard(__index_, __new_index);
134+
auto *__p = ::new (__storage_) _Ty{static_cast<_As &&>(__as)...};
135+
__sg.__dismiss();
136+
return *std::launder(__p);
115137
}
116138

117139
template <std::size_t _Ny, class... _As>
@@ -120,9 +142,10 @@ namespace stdexec {
120142
static_assert(_Ny < sizeof...(_Ts), "variant index is too large");
121143

122144
__destroy();
123-
::new (__storage_) __at<_Ny>{static_cast<_As &&>(__as)...};
124-
__index_ = _Ny;
125-
return *std::launder(reinterpret_cast<__at<_Ny> *>(__storage_));
145+
auto __sg = __mk_index_guard(__index_, _Ny);
146+
auto *__p = ::new (__storage_) __at<_Ny>{static_cast<_As &&>(__as)...};
147+
__sg.__dismiss();
148+
return *std::launder(__p);
126149
}
127150

128151
template <std::size_t _Ny, class _Fn, class... _As>
@@ -133,9 +156,11 @@ namespace stdexec {
133156
"callable does not return the correct type");
134157

135158
__destroy();
136-
::new (__storage_) __at<_Ny>(static_cast<_Fn &&>(__fn)(static_cast<_As &&>(__as)...));
137-
__index_ = _Ny;
138-
return *std::launder(reinterpret_cast<__at<_Ny> *>(__storage_));
159+
auto __sg = __mk_index_guard(__index_, _Ny);
160+
auto *__p = ::new (__storage_)
161+
__at<_Ny>(static_cast<_Fn &&>(__fn)(static_cast<_As &&>(__as)...));
162+
__sg.__dismiss();
163+
return *std::launder(__p);
139164
}
140165

141166
template <class _Fn, class... _As>

0 commit comments

Comments
 (0)