sandbox cleanup
Hal Murray
hmurray at megapathdsl.net
Thu Jun 1 22:48:54 UTC 2017
Would you please take a look at the code in ntp_sandbox.
It's 10 pages on my screen. Then it starts with the seccomp code. All one
big routine.
The whole area of processing user and group seems like a mess to me. Some of
the ulginess is probably because I don't really understand the Unix/POSIX
concepts in that area.
The problem that started things is a free at line 103 in ntp_sandbox. That
was trying to free a command line arg. Gary "fixed" it by copying the
parameter over so it could be freed.
The root of the problem is that APIs are complicated if things like
malloc/free are done across API boundaries. I think we should fix the API so
that sandbox never frees its arguments. I also think we should move the
:/group parsing from ntpd.c to ntp_sandbox.c
There may be other places with that API problem.
I think this area is appropriate for your skills and should be on the list
for 1.0
A few more quirks I noticed while looking at the code:
There is an #else at line 88 that looks bogus to me. The following test for
a NULL user should happen on all systems, not skipping Linux and Solaris.
Line 175:
if (user && initgroups(user, (gid_t)sw_gid)) {
user should have been tested above.
The cast to gid_t doesn't seem necessary. (static gid_t sw_gid;)
Why do we need to call initgroups? Does it need a free? ...
It also gets called at line 195
(I took a look in the man pages but didn't find enlightenment.)
--
These are my opinions. I hate spam.
More information about the devel
mailing list