[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:  needs_review
     Priority:  normal       |  Milestone:
    Component:  Tor          |    Version:
   Resolution:               |   Keywords:  tor-relay gsoc seccomp sandbox
Actual Points:               |  Parent ID:  #5756
       Points:               |
-----------------------------+--------------------------------------------

Comment (by ctoader):

 Replying to [comment:10 nickm]:
 I have added the changes and I hope everything is ok now, and we can
 merge. Thank you for helping with the items I've mentioned before, I would
 really struggle doing those myself. Please let me know if there is
 anything else.

 > QUick review:
 > * In prot_strings, you say: "strlen((char*) el->param)".  Why the cast?
 If we don't know it's a char*, we shouldn't be taking strlen() of it.
 (Are some of these an intptr? The type seems to be an intptr_t ... how do
 we know which ones are strings?)
 I left the cast for 2 reasons: originally parameter filtering had multiple
 types which included strings and integer values, and each filter item had
 a field saying how the initptr_t should be interpreted, but this was
 removed and now all values are interpreted as the address which holds the
 string; and second, the seccomp filter takes the address of the string
 pointer as a parameter so it would be a choice between casting it when the
 parameter is added to the filter or casting it when creating the parameter
 filter element.
 > * In prot_strings, string lengths should really be size_t.
 done
 > * Use tor_malloc() and tor_free() instead of malloc and free.
 done, except for [1] due to a compilation error introduced when using the
 macro; also the macro is not needed in that case because the value of the
 pointer is reassigned on the next line and value cannot be NULL.
 > * Use tor_strdup(), not strdup().
 done
 > * Every function should have documentation.
 done
 > * The hints argument to getaddrinfo is "const struct addrinfo hints *",
 not "struct addrinfo hints".  Shouldn't sandbox_getaddrinfo look the same
 way?
 done

 [1] https://github.com/cristiantoader/tor-gsoc-capabilities/blob/gsoc-cap-
 stage2/src/common/sandbox.c#L825

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9249#comment:13>
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