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