-
Notifications
You must be signed in to change notification settings - Fork 256
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
Fix permissions issues with sandbox mounts #1211
Fix permissions issues with sandbox mounts #1211
Conversation
404e7af
to
ec63fc6
Compare
// Temporarily set the umask of this process to 0 so that we can actually | ||
// make all dirs with os.ModePerm permissions. | ||
savedUmask := unix.Umask(0) | ||
if err := os.MkdirAll(sandboxSource, os.ModePerm); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not reset the umask if MkdirAll
returns non-nil error. Can we change this to something like:
savedumask := unix.Umask(0)
defer unix.Umask(savedUmask)
err := os.MkdirAll(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>
ec63fc6
to
aea3b96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Related work items: microsoft#1067, microsoft#1097, microsoft#1119, microsoft#1170, microsoft#1176, microsoft#1180, microsoft#1181, microsoft#1182, microsoft#1183, microsoft#1184, microsoft#1185, microsoft#1186, microsoft#1187, microsoft#1188, microsoft#1189, microsoft#1191, microsoft#1193, microsoft#1194, microsoft#1195, microsoft#1196, microsoft#1197, microsoft#1200, microsoft#1201, microsoft#1202, microsoft#1203, microsoft#1204, microsoft#1205, microsoft#1206, microsoft#1207, microsoft#1209, microsoft#1210, microsoft#1211, microsoft#1218, microsoft#1219, microsoft#1220, microsoft#1223
…_perm Fix permissions issues with sandbox mounts
An issue was reported where when a user went to write files as a non-privileged user in a sandboxMount mounted with r/w in a container, they'd get a permission denied error. This was because the underlying directories in the UVM were created with 0755 permissions so only a privileged user in the container could write to a r/w sandboxMount mounted in. This PR changes that behavior by creating the underlying directories with 0777 permissions.
Signed-off-by: Kathryn Baldauf kabaldau@microsoft.com