[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #9249 [Tor]: GSOC seccomp stage 2

#9249: GSOC seccomp stage 2
--------------------------------------------+-------------------------------
 Reporter:  ctoader                         |          Owner:  nickm
     Type:  enhancement                     |         Status:  new  
 Priority:  normal                          |      Milestone:       
Component:  Tor                             |        Version:       
 Keywords:  tor-relay gsoc seccomp sandbox  |         Parent:  #5756
   Points:                                  |   Actualpoints:       
--------------------------------------------+-------------------------------

Comment(by nickm):

 Quick review, as of dde3ed385bc9de8, since I think you asked for one:

   * LENGHT is mispelled.  (Probably it should be ARRAY_LENGTH, and it
 should be in util.h).
   * filter_dynamic should probably be static; nothing needs to use it
 outside of sandbox.c, right?
   * Exposing sandbox_static_cfg_t, pfd_elem, and sandbox_filter_func_t in
 the header seems wrong: those are not part of the public sandbox
 interface, right? They only make sense for seccomp2.
   * Some of the restrictions in sandbox.c don't make sense for security;
 we should at least document why we're doing them.  For example, why
 restrict clock_gettime() to CLOCK_MONOTONIC?  Why restrict time() to NULL?
   * Some of the restrictions in sandbox.c don't make sense given how the
 syscalls work.  (e.g., surely we're sometimes calling epoll_ctl with
 EPOLL_CTL_DEL!)
   * We probably want to restrict execve arguments in the same way we
 restrict open, openat, etc.
   * The structures and functions all need documentation.

 Not necessarily this time:
   * It's confusing to me how filter_dynamic is a linked list of so many
 different things.  Maybe once it's documented, it will make sense?
   * It seems excessive to use a separate mmap() for every prot_strdup()
 call.  Perhaps instead we should have a big mmap that fills up with
 strings, and which we mprotect when we're done.  This could wait until
 later though.

 Other than that, it's looking good....

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9249#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs