Skip to content

Commit f30a32e

Browse files
authored
Make sure altstack_size_ is a multiple of the system page size (#1883)
Summary: This PR modifies the logic that sets the value of `altstack_size_`. When deploying Pixie on a 7th gen AMI running bottle rocket, the value of `altstack_size_` was set to `MINSIGSTKSZ`. This value was 39616 and was not a multiple of the system page size. In this [line of code](https://github.com/pixie-io/pixie/blob/0888c38df2dc414a36cc84198c7b159c39eea0e0/src/common/signal/signal_action.cc#L149) `mprotect` was then applied on a non page aligned region since its address was set as `altstack_ + guard_size_ + altstack_size_` leading to an error. This region in `altstack_` was also not the tailing `guard_size_` area as [mmap](https://github.com/pixie-io/pixie/blob/0888c38df2dc414a36cc84198c7b159c39eea0e0/src/common/signal/signal_action.cc#L145) would have added extra bytes to `altstack_` in order to page align it. As per the docs, the address passed to `mprotect` needs to be aligned to a page boundary. The function in this PR makes sure that the value of `altstack_size_` is a multiple of the page size so that the address calculated/passed to `mprotect` is aligned to a page boundary and that it protects the tailing `guard_size_` region of `altstack_` Relevant Issues: Fixes #1882 Type of change: /kind bug Test Plan: Skaffolded pixie on EKS clusters with m7i.large, r7i.large instances and on a GKE cluster with e2-standard-4 nodes and saw the PEM start up --------- Signed-off-by: Kartik Pattaswamy <kpattaswamy@pixielabs.ai>
1 parent c399d12 commit f30a32e

File tree

1 file changed

+13
-1
lines changed

1 file changed

+13
-1
lines changed

src/common/signal/signal_action.h

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class SignalAction : public NotCopyable {
3535
public:
3636
SignalAction()
3737
: guard_size_(px::system::Config::GetInstance().PageSizeBytes()),
38-
altstack_size_(std::max(guard_size_ * 4, static_cast<size_t>(MINSIGSTKSZ))) {
38+
altstack_size_(DetermineAltStackSize()) {
3939
MapAndProtectStackMemory();
4040
InstallSigHandlers();
4141
}
@@ -71,6 +71,18 @@ class SignalAction : public NotCopyable {
7171
* Additionally, two guard pages will be allocated to bookend the usable area.
7272
*/
7373
const size_t altstack_size_;
74+
/**
75+
* Determine the number of bytes to allocate to altstack_size_.
76+
*/
77+
size_t DetermineAltStackSize() const {
78+
// The size of altstack_size_ should be at least 4 * guard size or size MINSIGSTKSZ if it is
79+
// greater. This size needs to be a multiple of the system page size.
80+
const size_t min_altstack_size = 4 * guard_size_;
81+
const size_t sig_page_count = IntRoundUpDivide(static_cast<size_t>(MINSIGSTKSZ), guard_size_);
82+
const size_t sig_stack_size = sig_page_count * guard_size_;
83+
84+
return std::max(min_altstack_size, sig_stack_size);
85+
}
7486
/**
7587
* Signal handlers will be installed for these signals which have a fatal outcome.
7688
*/

0 commit comments

Comments
 (0)