initrd : do initgroups on chroot #3521
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3521 +/- ##
=======================================
Coverage 19.47% 19.48%
=======================================
Files 231 231
Lines 50220 50220
=======================================
+ Hits 9781 9784 +3
+ Misses 39720 39718 -2
+ Partials 719 718 -1 ☔ View full report in Codecov by Sentry. |
shjala
changed the title
pkg/xen-tools/initrd/chroot2.c
Outdated
| return -1; | ||
| } | ||
|
|
||
| if (mount("proc", "/proc", "proc", 0, NULL) != 0) { |
There was a problem hiding this comment.
I think this mount is some leftover from former implementation... we are already mounting the /proc in the initrd script, as well as the other needed filesystems (/dev, /sys) before call the chroot. I think we can safely remove this mount and just expect that /proc will be ready like the others... as the POSIX chroot does, it won't mount any of these filesystems by himself...
There was a problem hiding this comment.
clone() is called with CLONE_NEWPID so AFAIR mounting it allows the launched process and all the subsequent childs have their own (limited) view of the /proc.
There was a problem hiding this comment.
now that you mentioned it I think we should create a new mnt namespace for the child, because this mount affects the parent process too.
I will send another PR for that.
check for errors on security sensitive calls and call initgroups() to remove root from the new uid supplementary groups. Signed-off-by: Shahriyar Jalayeri <[email protected]>
|
moved initgroups() before chroot() because it reads from |
check for errors on security sensitive calls and call initgroups() to remove root from the new uid supplementary groups.
This should go after #3512, so I keep it as WIP for now.