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

Set correct permissions of /tmp dir during GSC build #194

Closed
wants to merge 1 commit into from

Conversation

dimakuv
Copy link

@dimakuv dimakuv commented Apr 15, 2024

Description of the changes

The original app image may have changed the permissions of /tmp dir. At the same time, correct permissions are required for installation of packages during GSC build. This commit adds explicit chmod 1777 /tmp.

Fixes #193.

How to test this PR?

Looks like a reproducer would be like this:

FROM debian:12
RUN chmod 1777 /tmp
USER ${non_root_uid}:${non_root_gid}

But I haven't tested really.


This change is Reviewable

The original app image may have changed the permissions of `/tmp` dir.
At the same time, correct permissions are required for installation of
packages during GSC build. This commit adds explicit `chmod 1777 /tmp`.

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>
Copy link
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):
Looks invalid to me. First of all, wrong permissions or mismounted core directories is a problem with base image, not with gsc and we can't fix up for all possible problems. Second, debian:12 LGTM:

% docker run -it --rm --volume $PWD:/gramine debian:12 ls -ld /tmp    
drwxrwxrwt 2 root root 4096 Jun 12  2023 /tmp

So the problem is elsewhere. I'll ask in #193.


Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv and @woju)

a discussion (no related file):

Previously, woju (Wojtek Porczyk) wrote…

Looks invalid to me. First of all, wrong permissions or mismounted core directories is a problem with base image, not with gsc and we can't fix up for all possible problems. Second, debian:12 LGTM:

% docker run -it --rm --volume $PWD:/gramine debian:12 ls -ld /tmp    
drwxrwxrwt 2 root root 4096 Jun 12  2023 /tmp

So the problem is elsewhere. I'll ask in #193.

+1, this doesn't seem like a GSC bug, this PR seems like a workaround for a misconfiguration of someone's base image?



-- commits line 4 at r1:

The original app image may have changed the permissions of /tmp dir.

What apps are doing that? Reconfiguring the system globally? Sounds suspicious to me...

@dimakuv
Copy link
Author

dimakuv commented Aug 28, 2024

This is invalid, the problem was elsewhere. See the comments from other reviewers. Closing.

@dimakuv dimakuv closed this Aug 28, 2024
@dimakuv dimakuv deleted the dimakuv/always-chmod-tmp-to-1777 branch August 28, 2024 14:32
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.

GSC Build Image Couldn't Create Temporary File
3 participants