Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LibOS] Fix bug of RLIMIT_STACK being overwritten in child #1976

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dimakuv
Copy link
Contributor

@dimakuv dimakuv commented Aug 19, 2024

Description of the changes

Previously, the resource limit RLIMIT_STACK (maximum size of the process stack, in bytes) was overwritten in the child process, after fork. Thus, if the parent used setrlimit(RLIMIT_STACK), the effect of this was lost in the child. This PR fixes this bug, and adds a LibOS test to verify the correct behavior.

Fixes #1974

Noticed while reviewing #1973.

How to test this PR?

New LibOS test was added.


This change is Reviewable

This manifest option allows to modify the original `RLIMIT_NOFILE`
resource limit. There is *no* way to propagate this limit from the host;
this is a deliberate design choice.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
Previously, the resource limit `RLIMIT_STACK` (maximum size of the
process stack, in bytes) was overwritten in the child process, after
fork. Thus, if the parent used `setrlimit(RLIMIT_STACK)`, the effect of
this was lost in the child. This commit fixes this bug, and adds a LibOS
test to verify the correct behavior.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
Copy link
Contributor Author

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

a discussion (no related file):
This PR is based on top of #1973, thus requires that one to be merged first.



libos/src/libos_init.c line 303 at r1 (raw file):

        return 0;

    stack_size = ALLOC_ALIGN_UP(stack_size);

Note that stack size doesn't need to be aligned when setting the resource limit RLIMIT_STACK. That's what Linux does -- it doesn't silently adjust the stack size, instead Linux just writes whatever stack size the user supplies to it.

Thus I moved this alignment of the stack further down in this function.

@mkow mkow force-pushed the dimakuv/add-rlimit-noproc-manifest branch from aeacda6 to ca534ce Compare August 26, 2024 12:20
Base automatically changed from dimakuv/add-rlimit-noproc-manifest to master August 27, 2024 10:50
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.

[LibOS] RLIMIT_STACK is reset to default value in child process
1 participant