Add a new interceptors for cdbr(3) and cdbw(3) API from NetBSD

[krytarowski]

Summary

cdb - formats of the constant database.

cdbr, cdbr_open, cdbr_open_mem, cdbr_entries, cdbr_get, cdbr_find,
cdbr_close - constant database access methods.

cdbw_open, cdbw_put, cdbw_put_data, cdbw_put_key, cdbw_stable_seeder,
cdbw_output, cdbw_close - creates constant databases.

Add a dedicated test for this API.

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

7421 ↗

(On Diff #176266)

could you please extract this with comment and similar for write into functions?

7442 ↗

(On Diff #176266)

for WRITE maybe we can assume that system sets *base correctly.
however for READ, we don't know if values are corrupted, so it would be nice to be check them for shaddow, or just check that they point into the mmap_base

7456 ↗

(On Diff #176266)

function?

7515 ↗

(On Diff #176266)

Please extract functions here for structs as well.

[vitalybuka]

Assume WIP

[joerg]

I don't see why this interceptor needs to know about the internals of struct cdbr or struct cdbw at all. They are fully opaque. All memory accessed by users is explicitly sized as argument to the functions or returned with the size.

[krytarowski]

I was thinking about the same, whether there is point to sanitize internals of an opaque structure. I will simplify this.

[krytarowski]

• simplify

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

7421 ↗

(On Diff #176266)

it has been simplified, we no longer sanitize internals of opaque cdbr/cdbw struct.

7442 ↗

(On Diff #176266)

it has been simplified, we no longer sanitize internals of opaque cdbr/cdbw struct.

7456 ↗

(On Diff #176266)

it has been simplified, we no longer sanitize internals of opaque cdbr/cdbw struct.

7515 ↗

(On Diff #176266)

it has been simplified, we no longer sanitize internals of opaque cdbr/cdbw struct.

[vitalybuka]

In D55167#1324855, @joerg wrote:

I don't see why this interceptor needs to know about the internals of struct cdbr or struct cdbw at all. They are fully opaque. All memory accessed by users is explicitly sized as argument to the functions or returned with the size.

Then we give up on TSAN check of fieds, or msan check if caller will try to read particular fields

lib/sanitizer_common/sanitizer_common_interceptors.inc

8520 ↗

(On Diff #177581)

Why is this enough?
Are those pointers always point inside of sanitizer_cdbr?
e.g. sanitizer_cdbr::hash_base

If not than we are going to hide those accesses from TSAN and miss races.
Or if callers will read by those pointers, MSAN may falsely detect uninitialized values.

lib/sanitizer_common/sanitizer_platform_limits_netbsd.h

2259 ↗

(On Diff #177581)

if it is not sanitizing the fields, should the struct be replaced with __sanitizer_cdbr_sz?

[krytarowski]

In D55167#1326120, @vitalybuka wrote:

In D55167#1324855, @joerg wrote:

I don't see why this interceptor needs to know about the internals of struct cdbr or struct cdbw at all. They are fully opaque. All memory accessed by users is explicitly sized as argument to the functions or returned with the size.

Then we give up on TSAN check of fieds, or msan check if caller will try to read particular fields

These structs contain a pointer to a buffer and length.

Please have a look as the actual implementation.

https://nxr.netbsd.org/xref/src/common/lib/libc/cdb/cdbr.c#89

https://nxr.netbsd.org/xref/src/lib/libc/cdb/cdbw.c#71

There is another difficulty, that hash is a SLIST container (key_hash_head, hash) and I was unsure whether to sanitize it or skip.

Please help to make this clearer.

We certainly don't want to make this interceptor suboptimal.

[joerg]

In D55167#1326120, @vitalybuka wrote:

In D55167#1324855, @joerg wrote:

I don't see why this interceptor needs to know about the internals of struct cdbr or struct cdbw at all. They are fully opaque. All memory accessed by users is explicitly sized as argument to the functions or returned with the size.

Then we give up on TSAN check of fieds, or msan check if caller will try to read particular fields

As I said, the structures are fully opaque. The only defined interface is through the functions already wrapped. For the reader interface, TSAN doesn't make sense as idempotent beyond creation and destruction. For the writer, the only useful check for TSAN would be to rule out two threads using the same instance. That shouldn't require any access to the internals though.

[krytarowski]

lib/sanitizer_common/sanitizer_platform_limits_netbsd.h

2259 ↗

(On Diff #177581)

Actually its size is unknown to userland programs. I need to either hardcode its size or make a shadow variation.

[krytarowski]

• add WRITE operations in cdbw(3) interceptors
• use FD macros in cdbw_output()

[krytarowski]

OK, I think that this is now fine. cdbr(3) contains only read checks forsanitizer_cdbr . cdbw(3) contains read and write ones for sanitizer_cdbw.

[vitalybuka]

LGTM

lib/sanitizer_common/sanitizer_common_interceptors.inc

8525 ↗

(On Diff #177674)

could you please restructure piece after the REAL call similar to the function above?

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc

#define INIT_VIS                                                               \
   COMMON_INTERCEPT_FUNCTION(strunvis);                                         \
   COMMON_INTERCEPT_FUNCTION(strnunvis);                                        \
   COMMON_INTERCEPT_FUNCTION(strunvisx);                                        \
   COMMON_INTERCEPT_FUNCTION(strnunvisx)
 #else
 #define INIT_VIS
 #endif
 
+#if SANITIZER_INTERCEPT_CDB
+INTERCEPTOR(struct __sanitizer_cdbr *, cdbr_open, const char *path, int flags) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbr_open, path, flags);
+  if (path)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  struct __sanitizer_cdbr *cdbr = REAL(cdbr_open)(path, flags);
+  if (cdbr)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cdbr, sizeof(*cdbr));
+  return cdbr;
+}
+
+INTERCEPTOR(struct __sanitizer_cdbr *, cdbr_open_mem, void *base, SIZE_T size,
+  int flags, void (*unmap)(void *, void *, SIZE_T), void *cookie) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbr_open_mem, base, size, flags, unmap,
+    cookie);
+  if (base && size)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, base, size);
+  struct __sanitizer_cdbr *cdbr =
+    REAL(cdbr_open_mem)(base, size, flags, unmap, cookie);
+  if (cdbr)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cdbr, sizeof(*cdbr));
+  return cdbr;
+}
+
+INTERCEPTOR(u32, cdbr_entries, struct __sanitizer_cdbr *cdbr) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbr_entries, cdbr);
+  if (cdbr)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbr, sizeof(*cdbr));
+  return REAL(cdbr_entries)(cdbr);
+}
+
+INTERCEPTOR(int, cdbr_get, struct __sanitizer_cdbr *cdbr, u32 index,
+            const void **data, SIZE_T *datalen) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbr_get, cdbr, index, data, datalen);
+  if (cdbr)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbr, sizeof(*cdbr));
+  int ret = REAL(cdbr_get)(cdbr, index, data, datalen);
+  if (!ret) {
+    if (data)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, sizeof(*data));
+    if (datalen)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, datalen, sizeof(*datalen));
+    if (data && datalen)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, *data, *datalen);
+  }
+  return ret;
+}
+
+INTERCEPTOR(int, cdbr_find, struct __sanitizer_cdbr *cdbr, const void *key,
+            SIZE_T keylen, const void **data, SIZE_T *datalen) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbr_find, cdbr, key, keylen, data, datalen);
+  if (cdbr)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbr, sizeof(*cdbr));
+  if (key)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, key, keylen);
+  int ret = REAL(cdbr_find)(cdbr, key, keylen, data, datalen);
+  if (!ret) {
+    if (data)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, data, sizeof(*data));
+    if (datalen)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, datalen, sizeof(*datalen));
+    if (data && datalen)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, *data, *datalen);
+  }
+  return ret;
+}
+
+INTERCEPTOR(void, cdbr_close, struct __sanitizer_cdbr *cdbr) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbr_close, cdbr);
+  if (cdbr)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbr, sizeof(*cdbr));
+  REAL(cdbr_close)(cdbr);
+}
+
+INTERCEPTOR(struct __sanitizer_cdbw *, cdbw_open) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbw_open);
+  struct __sanitizer_cdbw *ret = REAL(cdbw_open)();
+  if (ret)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ret, sizeof(*ret));
+  return ret;
+}
+
+INTERCEPTOR(int, cdbw_put, struct __sanitizer_cdbw *cdbw, const void *key,
+  SIZE_T keylen, const void *data, SIZE_T datalen) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbw_put, cdbw, key, keylen, data, datalen);
+  if (cdbw)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbw, sizeof(*cdbw));
+  if (data && datalen)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, data, datalen);
+  if (key && keylen)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, key, keylen);
+  int ret = REAL(cdbw_put)(cdbw, key, keylen, data, datalen);
+  if (!ret && cdbw)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cdbw, sizeof(*cdbw));
+  return ret;
+}
+
+INTERCEPTOR(int, cdbw_put_data, struct __sanitizer_cdbw *cdbw, const void *data,
+  SIZE_T datalen, u32 *index) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbw_put_data, cdbw, data, datalen, index);
+  if (cdbw)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbw, sizeof(*cdbw));
+  if (data && datalen)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, data, datalen);
+  int ret = REAL(cdbw_put_data)(cdbw, data, datalen, index);
+  if (!ret) {
+    if (index)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, index, sizeof(*index));
+    if (cdbw)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cdbw, sizeof(*cdbw));
+  }
+  return ret;
+}
+
+INTERCEPTOR(int, cdbw_put_key, struct __sanitizer_cdbw *cdbw, const void *key,
+  SIZE_T keylen, u32 index) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbw_put_key, cdbw, key, keylen, index);
+  if (cdbw)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbw, sizeof(*cdbw));
+  if (key && keylen)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, key, keylen);
+  int ret = REAL(cdbw_put_key)(cdbw, key, keylen, index);
+  if (!ret && cdbw)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cdbw, sizeof(*cdbw));
+  return ret;
+}
+
+INTERCEPTOR(int, cdbw_output, struct __sanitizer_cdbw *cdbw, int output,
+  const char descr[16], u32 (*seedgen)(void)) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbw_output, cdbw, output, descr, seedgen);
+  COMMON_INTERCEPTOR_FD_ACCESS(ctx, output);
+  if (cdbw)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbw, sizeof(*cdbw));
+  if (descr)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, descr, internal_strnlen(descr, 16));
+  if (seedgen)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, (void *)seedgen, sizeof(seedgen));
+  int ret = REAL(cdbw_output)(cdbw, output, descr, seedgen);
+  if (!ret) {
+    if (cdbw)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, cdbw, sizeof(*cdbw));
+    if (output >= 0)
+      COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, output);
+  }
+  return ret;
+}
+
+INTERCEPTOR(void, cdbw_close, struct __sanitizer_cdbw *cdbw) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, cdbw_close, cdbw);
+  if (cdbw)
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, cdbw, sizeof(*cdbw));
+  REAL(cdbw_close)(cdbw);
+}
+
+#define INIT_CDB \
+  COMMON_INTERCEPT_FUNCTION(cdbr_open); \
+  COMMON_INTERCEPT_FUNCTION(cdbr_open_mem); \
+  COMMON_INTERCEPT_FUNCTION(cdbr_entries); \
+  COMMON_INTERCEPT_FUNCTION(cdbr_get); \
+  COMMON_INTERCEPT_FUNCTION(cdbr_find); \
+  COMMON_INTERCEPT_FUNCTION(cdbr_close); \
+  COMMON_INTERCEPT_FUNCTION(cdbw_open); \
+  COMMON_INTERCEPT_FUNCTION(cdbw_put); \
+  COMMON_INTERCEPT_FUNCTION(cdbw_put_data); \
+  COMMON_INTERCEPT_FUNCTION(cdbw_put_key); \
+  COMMON_INTERCEPT_FUNCTION(cdbw_output); \
+  COMMON_INTERCEPT_FUNCTION(cdbw_close)
+#else
+#define INIT_CDB
+#endif
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map =
       new ((void *)&metadata_mem) MetadataHashMap();  // NOLINT
 
   INIT_MMAP;
   INIT_MMAP64;
   INIT_TEXTDOMAIN;
static void InitializeCommonInterceptors() {
   INIT_SHA1;
   INIT_MD4;
   INIT_RMD160;
   INIT_MD5;
   INIT_FSEEK;
   INIT_MD2;
   INIT_SHA2;
   INIT_VIS;
+  INIT_CDB;
 
   INIT___PRINTF_CHK;
 }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h

 #define SANITIZER_INTERCEPT_CAPSICUM SI_FREEBSD
 #define SANITIZER_INTERCEPT_SHA1 SI_NETBSD
 #define SANITIZER_INTERCEPT_MD4 SI_NETBSD
 #define SANITIZER_INTERCEPT_RMD160 SI_NETBSD
 #define SANITIZER_INTERCEPT_MD5 SI_NETBSD
 #define SANITIZER_INTERCEPT_FSEEK (SI_NETBSD || SI_FREEBSD)
 #define SANITIZER_INTERCEPT_MD2 SI_NETBSD
 #define SANITIZER_INTERCEPT_SHA2 SI_NETBSD
+#define SANITIZER_INTERCEPT_CDB SI_NETBSD
 #define SANITIZER_INTERCEPT_VIS SI_NETBSD
 
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_limits_netbsd.h

 SHA2_EXTERN(256);
 SHA2_EXTERN(384);
 SHA2_EXTERN(512);
 
 #undef SHA2_EXTERN
 
 extern const int unvis_valid;
 extern const int unvis_validpush;
+
+struct __sanitizer_cdbr {
+  void (*unmap)(void *, void *, uptr);
+  void *cookie;
+  u8 *mmap_base;
+  uptr mmap_size;
+
+  u8 *hash_base;
+  u8 *offset_base;
+  u8 *data_base;
+
+  u32 data_size;
+  u32 entries;
+  u32 entries_index;
+  u32 seed;
+
+  u8 offset_size;
+  u8 index_size;
+
+  u32 entries_m;
+  u32 entries_index_m;
+  u8 entries_s1, entries_s2;
+  u8 entries_index_s1, entries_index_s2;
+};
+
+struct __sanitizer_cdbw {
+  uptr data_counter;
+  uptr data_allocated;
+  uptr data_size;
+  uptr *data_len;
+  void **data_ptr;
+  uptr hash_size;
+  void *hash;
+  uptr key_counter;
+};
 }  // namespace __sanitizer
 
 #define CHECK_TYPE_SIZE(TYPE) \
   COMPILER_CHECK(sizeof(__sanitizer_##TYPE) == sizeof(TYPE))
 
 #define CHECK_SIZE_AND_OFFSET(CLASS, MEMBER)                      \
   COMPILER_CHECK(sizeof(((__sanitizer_##CLASS *)NULL)->MEMBER) == \
                  sizeof(((CLASS *)NULL)->MEMBER));                \

compiler-rt/trunk/test/sanitizer_common/TestCases/NetBSD/cdb.cc

+// RUN: %clangxx -O0 -g %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include <sys/param.h>
+
+#include <sys/types.h>
+
+#include <sys/mman.h>
+#include <sys/stat.h>
+
+#include <assert.h>
+#include <cdbr.h>
+#include <cdbw.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+static char *name;
+
+const char data1[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
+const char data2[] = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17};
+const char key1[] = {0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27};
+const char key2[] = {0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37};
+
+void test_cdbw() {
+  uint32_t idx;
+
+  struct cdbw *cdbw = cdbw_open();
+  assert(cdbw);
+
+  int rv = cdbw_put_data(cdbw, data1, __arraycount(data1), &idx);
+  assert(!rv);
+
+  rv = cdbw_put_key(cdbw, key1, __arraycount(key1), idx);
+  assert(!rv);
+
+  rv = cdbw_put(cdbw, key2, __arraycount(key2), data2, __arraycount(data2));
+  assert(!rv);
+
+  name = strdup("/tmp/temp.XXXXXX");
+  assert(name);
+
+  name = mktemp(name);
+  assert(name);
+
+  int fd = open(name, O_RDWR | O_CREAT, 0644);
+  assert(fd != -1);
+
+  cdbw_output(cdbw, fd, "TEST1", cdbw_stable_seeder);
+
+  cdbw_close(cdbw);
+
+  rv = close(fd);
+  assert(rv != -1);
+}
+
+void test_cdbr1() {
+  struct cdbr *cdbr = cdbr_open(name, CDBR_DEFAULT);
+  assert(cdbr);
+
+  uint32_t idx = cdbr_entries(cdbr);
+  assert(idx > 0);
+  printf("entries: %" PRIu32 "\n", idx);
+
+  const void *data;
+  size_t data_len;
+  int rv = cdbr_get(cdbr, idx - 1, &data, &data_len);
+  assert(rv == 0);
+
+  printf("data: ");
+  for (size_t i = 0; i < data_len; i++)
+    printf("%02" PRIx8, ((uint8_t *)data)[i]);
+  printf("\n");
+
+  rv = cdbr_find(cdbr, key1, __arraycount(key1), &data, &data_len);
+
+  printf("data: ");
+  for (size_t i = 0; i < data_len; i++)
+    printf("%02" PRIx8, ((uint8_t *)data)[i]);
+  printf("\n");
+
+  cdbr_close(cdbr);
+}
+
+#define COOKIE ((void *)1)
+
+static void cdbr_unmap(void *cookie, void *base, size_t sz) {
+  assert(cookie == COOKIE);
+  int rv = munmap(base, sz);
+  assert(rv != -1);
+}
+
+void test_cdbr2() {
+  struct stat sb;
+
+  int fd = open(name, O_RDONLY);
+  assert(fd != -1);
+
+  int rv = fstat(fd, &sb);
+  assert(rv != -1);
+
+  size_t sz = sb.st_size;
+  assert(sz < SSIZE_MAX);
+
+  void *base = mmap(NULL, sz, PROT_READ, MAP_FILE | MAP_SHARED, fd, 0);
+  assert(base != MAP_FAILED);
+
+  rv = close(fd);
+  assert(rv != -1);
+
+  struct cdbr *cdbr = cdbr_open_mem(base, sz, CDBR_DEFAULT, cdbr_unmap, COOKIE);
+  assert(cdbr);
+
+  printf("entries: %" PRIu32 "\n", cdbr_entries(cdbr));
+
+  cdbr_close(cdbr);
+}
+
+int main(void) {
+  printf("cdb\n");
+
+  test_cdbw();
+  test_cdbr1();
+  test_cdbr2();
+
+  // CHECK: cdb
+  // CHECK: entries: 2
+  // CHECK: data: 1011121314151617
+  // CHECK: data: 0001020304050607
+  // CHECK: entries: 2
+
+  return 0;
+}