This is an archive of the discontinued LLVM Phabricator instance.

Add interceptors for the sysctl(3) API family from NetBSD
ClosedPublic

Authored by krytarowski on Nov 3 2018, 12:23 PM.

Details

Summary

Add new interceptors for:

  • sysctl
  • sysctlbyname
  • sysctlgetmibinfo
  • sysctlnametomib
  • asysctl
  • asysctlbyname

Cover the API with a new test file TestCases/NetBSD/sysctl.cc.

Diff Detail

Event Timeline

krytarowski created this revision.Nov 3 2018, 12:23 PM
vitalybuka added inline comments.Nov 7 2018, 1:37 PM
lib/sanitizer_common/sanitizer_common_interceptors.inc
7301

Why you don't check newp here?

7321

READ from namelenp?
same bellow

test/sanitizer_common/TestCases/NetBSD/sysctl.cc
102

Can you please use different prefixies in printfs so make reading errors easier?

  • apply fixes from review
  • add more checks
krytarowski marked 3 inline comments as done.Nov 7 2018, 4:02 PM
vitalybuka requested changes to this revision.Nov 7 2018, 4:29 PM
vitalybuka added inline comments.
lib/sanitizer_common/sanitizer_common_interceptors.inc
7289

Here and below

if (oldlenp) {
      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, oldlenp, sizeof(*oldlenp));
      if (oldp)
          COMMON_INTERCEPTOR_WRITE_RANGE(ctx, oldp, *oldlenp);
}
7338

*namelenp * sizeof(*name) ?

7340

this should be as well under "if (!res) {"

7342
if (csz) {
  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, csz, sizeof(*csz));
  if (cname)
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cname, *csz);
}
7361

should this be

*namelenp * sizeof(*name)

?

The number of elements in the mib array is given by the location specified by sizep before the call, and that location gives the number of entries copied after a suc- cessful call.

7382

The asysctl() and asysctlbyname() functions are wrappers for sysctl() and
sysctlbyname(). They return memory allocated with malloc(3) and resize
the buffer in a loop until all data fits.

if so we should not intercept asysctl and asysctlbyname

This revision now requires changes to proceed.Nov 7 2018, 4:29 PM
krytarowski added inline comments.Nov 7 2018, 5:54 PM
lib/sanitizer_common/sanitizer_common_interceptors.inc
7382

They are not macro wrappers, but rather complex standalone code.

https://nxr.netbsd.org/xref/src/lib/libc/gen/asysctl.c

asysctlbyname will break because int name[CTL_MAXNAME]; and u_int namelen will be unknown to sanitizers.

Trying to skip asysctl will be risky too, it might work but I will feel safer with an interceptor.

  • address comments from review
  • enhance tests
krytarowski marked 7 inline comments as done.Dec 3 2018, 3:24 PM
vitalybuka accepted this revision.Dec 3 2018, 4:26 PM

LGTM

lib/sanitizer_common/sanitizer_common_interceptors.inc
7327

needs clang-format

7334

{} is used inconsistently

This revision is now accepted and ready to land.Dec 3 2018, 4:26 PM
krytarowski marked 2 inline comments as done.Dec 3 2018, 5:51 PM
krytarowski added a subscriber: devnexen.
krytarowski added inline comments.
lib/sanitizer_common/sanitizer_common_interceptors.inc
7327

This is not a part of my diff.

CC: @devnexen the author

7334

This is not a part of my diff.

CC: @devnexen

This revision was automatically updated to reflect the committed changes.