Skip to content

Align Secret to the page size of the current system.#110

Merged
stouset merged 1 commit intomasterfrom
stephen/plwuourmyvlq
Apr 13, 2026
Merged

Align Secret to the page size of the current system.#110
stouset merged 1 commit intomasterfrom
stephen/plwuourmyvlq

Conversation

@stouset
Copy link
Copy Markdown
Owner

@stouset stouset commented Aug 14, 2025

Closes #109.

Multiple Secret<T> instances could previously share the same memory page. This would cause funlock to unlock multiple secrets at a time. With secrets aligned to page boundaries, we ensure that no two secrets can share the same page.

Additionally we had detected the multiple-unlock case in Windows, which returns an error when funlock is called on an already-unlocked page. We swallowed errors in this case, but with this change we no longer need to do so.

@stouset
Copy link
Copy Markdown
Owner Author

stouset commented Aug 14, 2025

I hate this approach. I'm going to play around and see if I can get a #[repr(align(…))] one to work.

@stouset stouset force-pushed the stephen/plwuourmyvlq branch from c7c8900 to ea99880 Compare August 14, 2025 23:27
@Pandapip1
Copy link
Copy Markdown

Any progress on this?

@stouset
Copy link
Copy Markdown
Owner Author

stouset commented Apr 13, 2026

I looked for awhile and wasn’t particularly happy with any of the options I came up with. Since I don’t have any better ideas at this point I’ll pick one and cut a release with a stopgap that isn’t ideal but at least closes the bug.

@stouset stouset force-pushed the stephen/plwuourmyvlq branch from ea99880 to 50b8cc3 Compare April 13, 2026 02:02
Closes #109.

Multiple `Secret<T>` instances could previously share the same memory
page. This would cause `munlock` to unlock multiple secrets at a time.
With secrets aligned to page boundaries, we ensure that no two secrets
can share the same page.

Additionally we had detected the multiple-unlock case in Windows, which
returns an error when `munlock` is called on an already-unlocked page.
We swallowed errors in this case, but with this change we no longer need
to do so.
@stouset stouset force-pushed the stephen/plwuourmyvlq branch from 50b8cc3 to f7da166 Compare April 13, 2026 21:30
@stouset stouset enabled auto-merge April 13, 2026 21:30
@stouset stouset merged commit 5d3277b into master Apr 13, 2026
4 checks passed
@stouset stouset deleted the stephen/plwuourmyvlq branch April 13, 2026 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accidental unlocking of secrets

2 participants