Page MenuHomeFreeBSD

Capsicumize traceroute
ClosedPublic

Authored by oshogbo on Jan 23 2017, 1:20 PM.
Tags
None
Referenced Files
F87127397: D9303.diff
Sat, Jun 29, 5:18 PM
F87080524: D9303.id24345.diff
Sat, Jun 29, 1:54 AM
F87077517: D9303.id25368.diff
Sat, Jun 29, 12:57 AM
F87070080: D9303.id24350.diff
Fri, Jun 28, 10:50 PM
Unknown Object (File)
Thu, Jun 27, 4:06 PM
Unknown Object (File)
Thu, Jun 20, 10:14 AM
Unknown Object (File)
Mon, Jun 3, 9:50 AM
Unknown Object (File)
Mon, Jun 3, 9:48 AM
Subscribers
None

Details

Summary

PR: 193973 (+ some of my changes)

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

oshogbo retitled this revision from to Capsicumize traceroute.
oshogbo updated this object.
oshogbo edited the test plan for this revision. (Show Details)
oshogbo added reviewers: bapt, emaste, cem, allanjude.

Please upload with context, e.g. git diff -U9999 or the svn command from https://wiki.freebsd.org/Phabricator

contrib/traceroute/traceroute.c
561

Is this message correct in the current libcasper world order? Should it be an error?

oshogbo edited edge metadata.

Thank you emaste@.

contrib/traceroute/traceroute.c
563

Given the if block above will exit, no need for a else block here

598

casper cannot be null at this point given there checks above that exits

1023

This will always be true there is a check above for that.

1858

maybe if would be more readable to have a bunch of defined earlier:

#if HAVE_LIBCAPSER
#define tcrt_gethostbyname(domain) cpa_gethostbyname(capdns, domain)
#else 
#define tcrt_gethostbyname(domain) cpa_gethostbyname(capdns, domain)
#endif

with all functions like that

contrib/traceroute/traceroute.c
230

we compile with -DHAVE_LIBCASPER; does this #if work?

370

maybe lose the extra space here, like you do with HAVE_LIBCASPER stack variables in fns?

561–562

errx instead of perror/exit?

1034

given that we already have cansandbox variable I'd prefer if we set if false if capsicum is not available, and drop the ENOSYS case

usr.sbin/traceroute/Makefile
3–4

it seems odd to have both of these; afaik we need only src.opts.mk?

oshogbo marked 2 inline comments as done.

Fixes proposed by emaste and bapt.

oshogbo added inline comments.
contrib/traceroute/traceroute.c
230

Not sure if I get this.

1858

When we will have libcaspermock this will not be needed any more.

oshogbo added inline comments.
contrib/traceroute/traceroute.c
230

After one more look I get this ;)

bapt edited edge metadata.
This revision is now accepted and ready to land.Feb 19 2017, 12:15 PM
emaste edited edge metadata.
emaste added inline comments.
contrib/traceroute/traceroute.c
565–566

what about initializing these with const char *types[] = { "NAME", "ADDR" };?

allanjude edited edge metadata.

tested fine here

This revision was automatically updated to reflect the committed changes.