Skip to content

Conversation

@richardleach
Copy link
Contributor

On Linux, anonymous files can be created with memfd_create(2). File sealing provides a mechanism for a process to ensure that the shared memory cannot be unexpectedly tampered with.

This commit adds the flags for this mechanism to the Fcntl module, from where they can be optionally exported.

See: https://man7.org/linux/man-pages/man2/memfd_create.2.html

@jkeenan
Copy link
Contributor

jkeenan commented Mar 9, 2024

On Linux, anonymous files can be created with memfd_create(2). File sealing provides a mechanism for a process to ensure that the shared memory cannot be unexpectedly tampered with.

I have to admit that I have a negative reaction whenever I see a pull request whose commit message reads like, On Linux we can do blah, blah, blah; therefore we should modify core like so. We're not supporting a Linux application; we're supporting a language on dozens of operating systems.

I'm particularly put off by this p.r.'s commit message, because in less than 60 seconds I was able to confirm that memfd_create appears on FreeBSD and NetBSD. (I'm not sure yet about OpenBSD.)

So I think should investigate other platforms, revise the commit message to reflect what we learn, then make the revised p.r. from a smoke-me/ branch so that our non-Linux smoke testing rigs give meaningful results. Thanks.

@richardleach
Copy link
Contributor Author

On Linux

I used #21695, since it had already been merged, as a template for both the change and the commit message. This seemed like a reasonable thing to do at the time, but I apologise for any offence caused and am happy to reword the commit message.

I'm not sure what a smoke-me branch would tell us, since Fnctl wraps all flag definitions in #ifdef directives, so the module will get the flag if the platform it is built on supports it, and won't if it doesn't.

@jkeenan
Copy link
Contributor

jkeenan commented Mar 9, 2024

I'm not sure what a smoke-me branch would tell us, since Fnctl wraps all flag definitions in #ifdef directives, so the module will get the flag if the platform it is built on supports it, and won't if it doesn't.

Precisely. A smoke-me branch would tell us whether we had done no harm (or not).

@nicomen
Copy link
Contributor

nicomen commented Mar 9, 2024

I'm not sure what a smoke-me branch would tell us, since Fnctl wraps all flag definitions in #ifdef directives, so the module will get the flag if the platform it is built on supports it, and won't if it doesn't.

Here's some info on the intention behind and concept of a smoke-me branch if that's what you were wondering about. https://perldoc.perl.org/perlgit#Using-a-smoke-me-branch-to-test-changes

On many Unix-like platforms, anonymous files can be created using
memfd_create(2). Where supported, File sealing provides a mechanism
whereby a process can ensure that the underlying shared memory cannot
be unexpectedly tampered with.

This commit adds the flags for this mechanism to the Fcntl module,
from where they can be optionally exported.

See, for example: https://man7.org/linux/man-pages/man2/memfd_create.2.html
@richardleach
Copy link
Contributor Author

@nicomen - I am familiar with smoke-me branches, but thanks for the link nonetheless.

@jkeenan - I've pushed smoke-me/hydahy/fcntl_seals

@richardleach
Copy link
Contributor Author

I've also updated the commit message within this PR.

@rwp0
Copy link
Contributor

rwp0 commented Mar 10, 2024

I've also updated the commit message within this PR.

Thanks, could you please update the PR description as well?

Uploading image.png…

@richardleach
Copy link
Contributor Author

Thanks, could you please also update the PR description as well?

For what benefit? It would make the subsequent flow of comments more difficult to understand.

@richardleach richardleach merged commit 4d119e9 into Perl:blead Mar 12, 2024
@richardleach richardleach deleted the hydahy/onkonk branch March 12, 2024 17: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.

5 participants