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

Mount a tmpfs at /dev/shm in the sandbox. #3263

Merged
merged 1 commit into from
Apr 11, 2020

Conversation

zenhack
Copy link
Collaborator

@zenhack zenhack commented Mar 10, 2020

...To get shm_open() and friends working.

Fixes #3196.

Note that the tests are currently not passing on my machine -- for some reason shm_open is returning ENOSYS. Based on the links @abliss posted at:

#3196 (comment)

and the fact that there are two alternate implementations of shm_open, depending on build flags:

https://sourceware.org/git/?p=glibc.git;a=blob;f=rt/shm_open.c;h=b4623d3cdaf5a26e4fd795a866c12aa1b29eaa9a;hb=HEAD

...one possible explanation is that my system's glibc is built without support for this. So I'd be curious as to whether the tests pass for others. Grepping through the postgres source suggests it has some fallbacks if shm_open is not available, so another possibility is that the errors @orblivion saw re: /dev/shm weren't actually going through shm_open.

We should investigate and work out what's going on there before merging this.

@ocdtrekkie ocdtrekkie added the app-platform App/Sandstorm integration features label Mar 10, 2020
@zenhack
Copy link
Collaborator Author

zenhack commented Mar 11, 2020

Looks like it's failing the same way in CI as it s on my machine:

https://github.com/sandstorm-io/sandstorm/pull/3263/checks?check_run_id=499117544#step:9:1407

@kentonv
Copy link
Member

kentonv commented Mar 15, 2020

According to the man page for shm_open, you need to link with librt (-lrt), which I think test-app currently isn't. Maybe you're seeing a stub function in the base glibc that would be overridden if you linked with librt?

@zenhack
Copy link
Collaborator Author

zenhack commented Mar 15, 2020

I added -lrt in the Makefile (I got a linker error without it), so I don't think that's it, unless the test app uses different LDFLAGS than everything else? (but then I don't understand why adding in the makefile would have fixed anything, so that seems unlikely).

@kentonv
Copy link
Member

kentonv commented Mar 20, 2020

Oh woops, I thought I looked to see if you had added it and didn't see it, but given it's the very first thing in the diff I don't know how I could have missed it.

But... I dunno then.

@ocdtrekkie
Copy link
Collaborator

As a note, this will also help get .NET Core working on Sandstorm: dotnet/runtime#2534 (comment)

...To get shm_open() and friends working.

Fixes sandstorm-io#3196.
@zenhack
Copy link
Collaborator Author

zenhack commented Mar 27, 2020

Fixed. Just had a bogus leading slash in front of dev/shm. Derp.

@zenhack zenhack changed the title WIP: Mount a tmpfs at /dev/shm in the sandbox. Mount a tmpfs at /dev/shm in the sandbox. Mar 27, 2020
@zenhack
Copy link
Collaborator Author

zenhack commented Mar 28, 2020

@ocdtrekkie, feel free to tag this one ready-for-review.

@ocdtrekkie ocdtrekkie added the ready-for-review We think this is ready for review label Mar 28, 2020
// so we don't have to strictly separate their storage capacity. We could mount
// a single tmpfs somewhere invisible, create subdirectories, and then bind-mount
// them to their final destinations.
KJ_SYSCALL(mkdir("dev/shm", 0700));
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the /dev filesystem itself is created with 0755 permission, any particular reason to use 0700 here? I guess it ultimately doesn't mater since there's only one user in the sandbox and they own everything?

@zenhack
Copy link
Collaborator Author

zenhack commented Apr 5, 2020 via email

@kentonv kentonv merged commit 6d57b3c into sandstorm-io:master Apr 11, 2020
@ocdtrekkie ocdtrekkie removed the ready-for-review We think this is ready for review label Apr 11, 2020
@zenhack zenhack deleted the dev-shm branch December 28, 2020 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-platform App/Sandstorm integration features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider putting a tmpfs at /dev/shm, to support shm_open and friends.
3 participants