gramineproject / graphene

Graphene / Graphene-SGX - a library OS for Linux multi-process applications, with Intel SGX support
https://grapheneproject.io
GNU Lesser General Public License v3.0
771 stars 260 forks source link

Support for "setsid" system call #2638

Closed ravikumarbhattiprolu closed 3 years ago

ravikumarbhattiprolu commented 3 years ago

Description of the problem

Enhancement Request - Need support for "setsid" system call to enable proper handling of signals inside the docker and also to avoid zombie processes when docker run is killed with SIGTERM. This is needed for proper handling of gsc containers.

Saw that it is in the todo list. Can the priority be increased?

Steps to reproduce

Expected results

Actual results

boryspoplawski commented 3 years ago

What do you mean? You cannot run docker inside Graphene and if you are talking about Graphene inside a docker container, then Graphene emulated setsid (or lack of it) does not matter at all.

ravikumarbhattiprolu commented 3 years ago

We are able to run gsc containers. However, in one of our containers, we use dumb-init as entry point to docker and it uses setsid to start the actual program. Since setsid is not yet supported in graphene, we have to modify the docker to change entry point.

boryspoplawski commented 3 years ago

There are no plans to support setsid. There is no notion of terminal in Graphene (and probably never be) so implementing setsid makes no sense. P.S. I don't know why would you use setsid in non-Graphene case in the scenario you described.

boryspoplawski commented 3 years ago

Saw that it is in the todo list.

Where did you see that exactly?

ravikumarbhattiprolu commented 3 years ago

The existing docker is implemented with dumb init as entry point as per https://engineeringblog.yelp.com/2016/01/dumb-init-an-init-for-docker.html We are trying to reconfigure the same docker into GSC and there we need support for setsid system call. I remember seeing a reference to setsid in one of the documents. I will check again.

ravikumarbhattiprolu commented 3 years ago

In the file LibOS/shim/src/sys/shim_getpid.c I saw below:

long shim_do_setsid(void) { / TODO: currently we do not support session management. / return -ENOSYS; }

boryspoplawski commented 3 years ago

The existing docker is implemented with dumb init as entry point as per https://engineeringblog.yelp.com/2016/01/dumb-init-an-init-for-docker.html We are trying to reconfigure the same docker into GSC and there we need support for setsid system call.

From dumb-init docs:

It launches a single process and then proxies all received signals to a session rooted at that child process.

This is weird. If you want to send a signal to the whole process tree rooted at pid 1 in container, just use a process group. No need for proxying signals... Also you can normally send all signals to pid1 in container from outside, the restrictions only apply to signals from inside. Generally to me it seems like this dumb-init solves non-existing issues.

/ TODO: currently we do not support session management. /

That TODO is missleading, there are no plans for it.

boryspoplawski commented 3 years ago

BTW, you cannot inject host-level signals into a Graphene process (except for one time SIGTERM injection, if you opt-in for that in manifest).

mkow commented 3 years ago

@ravikumarbhattiprolu: Seems that Borys answered all your issues and there's nothing on our side to do, so I'm closing it.