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

Make sure unveil(2) is properly locked #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tnias
Copy link

@tnias tnias commented Oct 12, 2024

This change ensures that unveil always gets locked (by passing two NULL arguments) and at least one path ("/" with no permissions) got added to unveil before that.

When no existing paths were registered with unveil(2), the filesystem access is not restricted by it. Locking it in that state only prevents us from configuring it further.

Notes on what I tried out.

Observations

Unveil gets locked, when we pass something via the '-f' flag. But it does not (or
only partially) when we do not.

With custom config and no default config

$ touch empty.config
$ ktrace ./endlessh -f empty.config
^C
$ kdump | grep unveil
 81470 endlessh CALL  unveil(0x32be4cda6d7,0x32be4cda4ea)
 81470 endlessh RET   unveil -1 errno 2 No such file or directory
 81470 endlessh STRU  promise="inet stdio rpath unveil"
 81470 endlessh CALL  unveil(0x6fd5478a252e,0x32be4cda4ea)
 81470 endlessh RET   unveil 0
 81470 endlessh CALL  unveil(0,0)
 81470 endlessh RET   unveil 0

While ps -O state reports: 81470 I+pU p4 0:00.00 ./endlessh -f empty.config.

Notice the U. (this is good)

Quote of ps(1):

U       The process has unveiled, and unveil(2) is now
        locked.
u       The process has unveiled, but not yet locked
        unveil(2) (could be a program error).

Without custom config and no default config

$ ls /etc/endlessh
ls: /etc/endlessh: No such file or directory
$ ktrace ./endlessh
^C
$ kdump| grep unveil
 17997 endlessh CALL  unveil(0xf1b60b146d7,0xf1b60b144ea)
 17997 endlessh RET   unveil -1 errno 2 No such file or directory
 17997 endlessh STRU  promise="inet stdio rpath unveil"

While ps reports: 17997 S+p p4 0:00.00 ./endlessh.

Notice no U or u. (this is not good)

Without custom config and the default config directory

# mkdir /etc/endlessh
$ ktrace ./endlessh
^C
$ kdump| grep unveil
 10140 endlessh CALL  unveil(0xe5d37d676d7,0xe5d37d674ea)
 10140 endlessh RET   unveil 0
 10140 endlessh STRU  promise="inet stdio rpath unveil"

While ps reports: 10140 S+pu p4 0:00.00 ./endlessh

Notice only u. (this is not good)

After patch (without default config and no default config)

$ ktrace ./endlessh
^C
$ kdump| grep unveil
 83117 endlessh CALL  unveil(0x2dbba65d486,0x2dbba65d396)
 83117 endlessh RET   unveil 0
 83117 endlessh CALL  unveil(0x2dbba65d6da,0x2dbba65d4ed)
 83117 endlessh RET   unveil -1 errno 2 No such file or directory
 83117 endlessh STRU  promise="inet stdio rpath unveil"
 83117 endlessh CALL  unveil(0,0)
 83117 endlessh RET   unveil 0

While ps reports: 83117 S+pU p4 0:00.00 ./endlessh (with the U. yay)

This change ensures that unveil always gets locked (by passing two NULL
arguments) and at least one path ("/" with no permissions) got added to
unveil before that.

When no existing paths were registered with unveil(2), the filesystem
access is not restricted by it. Locking it in that state only prevents
us from configuring it further.
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.

1 participant