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

Race condition in file permission #1152

Open
webmake opened this issue Nov 21, 2020 · 7 comments
Open

Race condition in file permission #1152

webmake opened this issue Nov 21, 2020 · 7 comments
Labels

Comments

@webmake
Copy link

webmake commented Nov 21, 2020

Hello,

it's quite similar to #1062 but for us happened error with bad permissions, because one process puts content, and other tries to read it and gets Key file "file:///tmp/****.key" permissions are not correct, recommend changing to 600 or 660 instead of 644
I think it really deserved locking..

@Sephster
Copy link
Member

Can you give a little more information about how it is you have two processes interacting with the file? I haven't come across this before. Thanks!

@webmake
Copy link
Author

webmake commented Nov 23, 2020

It's very hard to reproduce, because it happened once, and I suspect that it was two identical time http requests and concurring cpu cycles.
You can fake this situation by adding additional sleep before changing permission, and you will see that. I was told that it could be io wait, so thats why it occurred.

first process ops:

file_exists
file_put_contents
(io wait ??)
chmod

second process ops:

file_exists
return
strpos
file_exists
is_readable
$keyPermissionsCheck === true
fileperms

@Sephster
Copy link
Member

@webmake sorry for the delayed response. Why do you have two PHP instances reading the file?

@webmake
Copy link
Author

webmake commented Jan 14, 2021

This is http requests, so aren't controlled how many in parallel

@Sephster
Copy link
Member

I've done a bit more reading around this and realise that this could occur because Apache/NginX can spawn concurrent processes that cause this kind of race condition. Apologies as I originally thought you might have had two servers accessing the same file share or something.

I'm at a loss as to how to fix this to be honest. We could put a file lock but PHP file locking is anything but certain because it is only advisory.

Is this the first time you've come across this? It is the first time to my knowledge that it has been reported.

I will likely remove the file permissions checks in the next major release as I think the package probably shouldn't check this anyways (and it doesn't work on non unix based systems).

If this is as rare as I think it is, I'm inclined to just leave this as the effort doesn't outweigh the benefit. Would welcome your thoughts on this.

@Sephster Sephster added the Bug label Jan 15, 2021
@webmake
Copy link
Author

webmake commented Jan 15, 2021

We are encountering each time after deployment at higher load, (9cases per month). There would be solution by using locks or another file that is is ongoing save procedure

@Sephster
Copy link
Member

Thanks @webmake - that is more frequent than I was expecting. I will keep this open and look to implement a simple flock implementation which should resolve the issue. Thanks for reporting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants