Skip to content

posix: implement memfd and file sealing - #418

Merged
qookei merged 11 commits into
managarm:masterfrom
no92:memfd
Feb 12, 2022
Merged

qookei merged 11 commits into
managarm:masterfrom
no92:memfd

Conversation

@no92

@no92 no92 commented Feb 4, 2022

Copy link
Copy Markdown
Member

The MemoryFile used here is an adapted version of the same struct in tmpfs, stripped of some functionality not yet needed.

@no92

no92 commented Feb 4, 2022

Copy link
Copy Markdown
Member Author

CI is failing right now because it depends on a definition of F_SEAL_SEAL, which is included in the mlibc PR.

Comment thread posix/subsystem/src/file.hpp Outdated
Comment thread posix/subsystem/src/memfd.hpp Outdated
@no92

no92 commented Feb 6, 2022

Copy link
Copy Markdown
Member Author

Addressed the issues raised above.

@no92

no92 commented Feb 6, 2022

Copy link
Copy Markdown
Member Author

Dropped some noops that are better served by File.

@no92
no92 marked this pull request as draft February 6, 2022 20:30
@no92

no92 commented Feb 6, 2022

Copy link
Copy Markdown
Member Author

Blocked on mesa not compiling with memfd present, due to it making direct Linux syscalls.

@Dennisbonke

Copy link
Copy Markdown
Member

Blocked on mesa not compiling with memfd present, due to it making direct Linux syscalls.

I think I might be able to patch that out but with weston 10 crashing consistently I'm unable to properly test that. Feel free to ping me on discord at a later date so I can look at it.

Comment thread posix/subsystem/src/main.cpp Outdated
if(logRequests)
std::cout << "posix: MEMFD_CREATE " << req->name() << std::endl;

std::cout << "posix: MEMFD_CREATE ignores some flags" << std::endl;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we return EINVAL for unsupported flags?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

According to the man page we should yes.

@no92

no92 commented Feb 10, 2022

Copy link
Copy Markdown
Member Author

Pushed commits that fixed Dennis' issues. Weston 10 now works for both of us.

@no92
no92 marked this pull request as ready for review February 10, 2022 16:32

@avdgrinten avdgrinten left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See below. Also, this PR contains commits that are not related to memfd and file sealing; this should be avoided.

Comment thread posix/subsystem/src/process.cpp
Comment thread posix/init/src/stage2.cpp Outdated
no92 and others added 5 commits February 11, 2022 21:16
@no92

no92 commented Feb 11, 2022

Copy link
Copy Markdown
Member Author

Addressed the issues raised above.

@Dennisbonke

Copy link
Copy Markdown
Member

LGTM now

@avdgrinten avdgrinten left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One small comment, can be merged afterwards.

Comment thread posix/subsystem/src/memfd.hpp Outdated

@avdgrinten avdgrinten left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM now.

@qookei
qookei merged commit 6506b04 into managarm:master Feb 12, 2022
@no92
no92 deleted the memfd branch February 12, 2022 19:57
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.

4 participants