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

Remove the command-level process limit #41

Closed
james-perretta opened this issue Mar 12, 2020 · 1 comment · Fixed by #44 or #45
Closed

Remove the command-level process limit #41

james-perretta opened this issue Mar 12, 2020 · 1 comment · Fixed by #44 or #45

Comments

@james-perretta
Copy link
Contributor

james-perretta commented Mar 12, 2020

This will let us do away with the shenanigans of modifying the "autograder" user's UID, but at the cost of only being able to limit total processes for the container. We believe that this is not a significant loss and will relieve the recurring process limit headaches we've had to deal with.

Problems with our current approach:

  • Using usermod -u on the "autograder" user does NOT update permissions on files owned by "autograder" outside of the home directory.
  • Calling usermod -u can take a long time if there are a lot of files in /home/autograder
  • Since ulimit applies to the user rather than the process, setting a per-command limit isn't truly a per-command limit (a service started in the background would count towards the limit for other tests).

Potential (non-)solutions and their problems:

  • X Cgroups: Mounting cgroups in a container requires disabling certain security features. See Investigate using cgroups in cmd_runner.py to limit processes #38
  • Ptrace: Needs research, but could be used to completely block forking for a command. We lose some granularity this way, but this could satisfy users who's goal is to stop all forks.
    • A ptrace fork-blocker could be written and used separately by users without changing this library
@james-perretta james-perretta added this to To do in Version 5.0.0 Apr 28, 2020
@james-perretta james-perretta moved this from To do to In progress in Version 5.0.0 Apr 28, 2020
@james-perretta james-perretta moved this from In progress to To do in Version 5.0.0 Apr 28, 2020
@james-perretta james-perretta changed the title Consider removing the command-level process limit Removing the command-level process limit Apr 28, 2020
@james-perretta james-perretta moved this from To do to In progress in Version 5.0.0 Apr 28, 2020
@james-perretta james-perretta changed the title Removing the command-level process limit Remove the command-level process limit Apr 28, 2020
Version 5.0.0 automation moved this from In progress to Done Apr 28, 2020
Version 5.0.0 automation moved this from Done to In progress Apr 29, 2020
@james-perretta
Copy link
Contributor Author

PR did not remove max_num_processes from AutograderSandbox.run_command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Version 5.0.0
  
Done
1 participant