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

APC_MEMPROTECT macro has no tests, won't work with apc.serializer=default #457

Open
TysonAndre opened this issue Oct 25, 2022 · 2 comments

Comments

@TysonAndre
Copy link
Contributor

TysonAndre commented Oct 25, 2022

Currently, I assume it'd be manually set with -DAPC_MEMPROTECT=1, but I wouldn't recommend that since I assume nobody has used this in years?

It may be of use to have a way to mark the returned pointers as read-only (to crash as early as possible instead of having applications misbehave when memory is being used in a way it shouldn't, e.g. due to bugs in pecls or unserializers)

  • That type of bug seems really, really unlikely in practice, and apcu always copies arrays and strings when reading from shared memory
@TysonAndre
Copy link
Contributor Author

The SHM_RDONLY flag is a no-op in TSRM/tsrm_win32.c, so it might also make sense to error and fail with a message saying it does nothing on windows

@TysonAndre
Copy link
Contributor Author

The SHM_RDONLY flag is a no-op in TSRM/tsrm_win32.c, so it might also make sense to error and fail with a message saying it does nothing on windows

Replacing it with VirtualProtect and mprotect similar to what opcache.protect_memory=1 does seems like a good alternative, which would work on both mmap and shmat with anonymous shared mappings (default, /dev/zero) where APC_MEMPROTECT only worked with named files

Refer to https://github.com/TysonAndre/immutable_cache-pecl/pull/11/files for how this might be implemented (though I expect APCu would be more complicated, e.g. have edge cases for cache clears) - this would be done by moving locks/mutexes into a separate shared memory region (go from n -> n+1 regions) (because acquiring mutexes requires the ability to write to shared memory)

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

No branches or pull requests

1 participant