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

Pseudoterminals failing in sandbox #554

Closed
MarcKe opened this issue Feb 21, 2024 · 3 comments
Closed

Pseudoterminals failing in sandbox #554

MarcKe opened this issue Feb 21, 2024 · 3 comments

Comments

@MarcKe
Copy link
Contributor

MarcKe commented Feb 21, 2024

Hi,

I have a question, potential improvement for the sandbox in bob.
Pseudoterminals require /dev/ptmx. Simply adding /dev/ptmx to the mounts of the sandbox is not enough.
For some weird reason the bind mount to /dev/ptmx is useless ("no such file or directory" error, tested with debian 11, arch linux with kernel 6.6.3). Linux suggests using mount --bind /dev/pts/ptmx /dev/ptmx, but the issue is here that most often the access rights of /dev/pts/ptmx are 000. This is a problem if you have no rights to change that.
(https://github.com/torvalds/linux/blob/master/Documentation/filesystems/devpts.rst)

A simple solution seems to be to add a separate devpts to the sandbox.
I added

CreateTarget("dev/pts", true);
CHECK_CALL(mount("devpts", "dev/pts", "devpts", MS_NOSUID | MS_NOEXEC, "ptmxmode=0666"));
CreateFile("dev/ptmx");
CHECK_CALL(mount("dev/pts/ptmx", "dev/ptmx", NULL, MS_REC | MS_BIND, NULL));

to SetupDevices() of the sandbox helper. "/dev/pts" needs to be removed from the recipe mounts of course.

What are your opinions here?

@jkloetzke
Copy link
Member

Sounds reasonable. Also bubblewrap seems to mount a new instance of devpts. But they symlink /dev/ptmx to /dev/pts/ptmx (see here). I would also go this route instead of bind mounting.

Would you mind creating a pull request? I'm currently preparing the 0.24 release but this can certainly ship with it...

@MarcKe
Copy link
Contributor Author

MarcKe commented Feb 22, 2024

Sounds reasonable. Also bubblewrap seems to mount a new instance of devpts. But they symlink /dev/ptmx to /dev/pts/ptmx (see here). I would also go this route instead of bind mounting.

Would you mind creating a pull request? I'm currently preparing the 0.24 release but this can certainly ship with it...

Indeed, a symlink sounds more reasonable here.
I will create a pull request.

@MarcKe MarcKe changed the title Pseudoterminals in Sandbox failing Pseudoterminals failing in sandbox Feb 22, 2024
@jkloetzke
Copy link
Member

Fixed by #555

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

2 participants