NVIDIA / nvidia-persistenced

NVIDIA driver persistence daemon
MIT License
55 stars 13 forks source link

Call to `setgroups()` missing in privilege drop logic #13

Open mgerstner opened 3 weeks ago

mgerstner commented 3 weeks ago

In the daemonize() function there is a call to setgroups() missing to drop any potential ancilliary group memberships.

This should not be a problem on most distributions, but for completeness it should be added to the code.

aritger commented 3 weeks ago

Thanks for the suggestion. To make sure I understand, since I'm out of practice in this area, is the recommendation to do

setgroups(0, NULL);

to drop any "supplementary groups"?

I imagine that should be done earlier in the function, before we check getuid/getgid? Maybe as soon as after creating a new session with setsid()? https://github.com/NVIDIA/nvidia-persistenced/blob/main/nvidia-persistenced.c#L805

mgerstner commented 3 weeks ago

I would drop all supplementary groups, yes. The correct time is to do it before calling setuid(). I don't believe it matters if it is done before or after setgid().