Add user-defined callback on write() calls.

[skerner]

Summary

Add dfsan_set_write_callback(), which sets a callback to be invoked when a write() call is invoked within DFSan instrumented code.

[skerner]

Nits.

[skerner]

Peter,

Ready for review.  I could not decide if it was worth using an atomic swap to install the callback pointer in lib/dfsan/dfsan.cc .  I put in a TODO, but I am not sure it is worth trying to support multiple simultaneous calls to that method.  Anyone who does such a thing is almost certainly not using the function in a reasonable way.

Sam

[pcc]

lib/dfsan/dfsan.cc

261 ↗

(On Diff #8315)

I don't think this approach is correct. We can't store a pointer to the callback function (which uses the instrumented ABI) and call it from the dfsan runtime library using the uninstrumented ABI, as the labels received by the callback function will be wrong (nor is this guaranteed to work at all).

We have a trampoline mechanism for safely calling instrumented ABI functions from the dfsan runtime library. Please take a look at how the custom functions pthread_create and dl_iterate_phdr are implemented.

When you have done this, you should be able to improve your test to show that the values passed to the write callback have the same labels as those passed to the write function.

lib/dfsan/dfsan_custom.cc

808

The signature of this function is incorrect. It should accept labels for each of the parameters and a pointer to the return label.

810

It would probably be simpler to call the callback unconditionally and let it check buf for labels. This would also allow the client to observe unlabelled writes if it needs to for whatever reason.

816

You could exit early here.

test/dfsan/write_callback.c

2

Maybe we could check that these files have the contents we expect?

[skerner]

Comments addressed.

It is not obvious to me what the label on the return value of write() would be. Please explicitly look at what I did (dfsan_custom.c, line 849, in __dfsw_write() ) and let me know if you agree with it.

lib/dfsan/dfsan.cc

261 ↗

(On Diff #8315)

Changed the code to use the trampoline mechanism. Improved tests.

lib/dfsan/dfsan_custom.cc

808

Done

810

Good point. Done.

816

Code removed.

test/dfsan/write_callback.c

2

It would be reasonable to assume that calls to write() do not fail in
tests. But write() may only write some of the bytes it is passed.
For example:

char buf[] = "123456789";
int bytes_written = write(fd, buf, 10);

If (for whatever reason) only 5 bytes are written, write may return 5.
I doubt it would do so on a local file system, but I don't want to
assume something that is not in the spec.

I considered adding retries to writes:

char buf[] = "123456789";

char* cur = buf;
int bytes_left = strlen(buf);
while ( bytes_left > 0 ) {

int result = write(fd, cur, bytes_left);

if (result < 0) ERROR HANDLER

bytes_left -= result;

}

... but now the number of callbacks tests must expect changes based on
how many times write() ends up being called.

[skerner]

test/dfsan/write_callback.c

2

Please ignore the above comment. I wrote it before realizing that its premise is flawed, and wrote the rest in a way that does check that the file is written.

[pcc]

lib/dfsan/dfsan_custom.cc

821

This isn't the correct type for write_callback -- I would use void* here.

832

This isn't necessarily always going to be the function pointer that was originally passed to dfsan_set_write_callback -- in theory, if the callee is known, we could generate the trampoline such that it calls the callee directly and the second argument is null. It would probably be simplest not to return anything from this function.

854

We haven't traditionally labelled the return value of system call functions according to their inputs, even with strict data dependencies turned on. The general justification for this is that system calls involve so much process-external state that we shouldn't even attempt to model their data flow beyond simply zero labeling their outputs.

I'm also not sure if this labelling is correct (for example, it reads the buffer's labels, even though the success or failure of a write is typically not dependent on the data being written).

test/dfsan/write_callback.c

83

I was imagining that the test could use something like FileCheck to check the program's output, rather than having the program check its own output. This would reduce the complexity of the test program and permit external validation of the program's behavior -- the wrappers could conceivably be lying to the program about the file system state.

[skerner]

Addressed review comments.

lib/dfsan/dfsan_custom.cc

821

Done.

832

Changed to not return the previous callback.

854

The general justification for this is that system calls involve so much
process-external state that we shouldn't even attempt to model their
data flow beyond simply zero labeling their outputs.

Makes sense. Removed.

test/dfsan/write_callback.c

83

Good points. Changed the code to use FileCheck.

[pcc]

LGTM

[pcc]

r207131

include/sanitizer/dfsan_interface.h

struct dfsan_label_info {
   dfsan_label l1;
   dfsan_label l2;
 
   // Fields for base labels.
   const char *desc;
   void *userdata;
 };
 
+/// Signature of the callback argument to dfsan_set_write_callback().
+typedef void (*dfsan_write_callback_t)(int fd, const void *buf, size_t count);
+
 /// Computes the union of \c l1 and \c l2, possibly creating a union label in
 /// the process.
 dfsan_label dfsan_union(dfsan_label l1, dfsan_label l2);
 
 /// Creates and returns a base label with the given description and user data.
 dfsan_label dfsan_create_label(const char *desc, void *userdata);
 
 /// Sets the label for each address in [addr,addr+size) to \c label.

 
 /// If the given label label contains a label with the description desc, returns
 /// that label, else returns 0.
 dfsan_label dfsan_has_label_with_desc(dfsan_label label, const char *desc);
 
 /// Returns the number of labels allocated.
 size_t dfsan_get_label_count(void);
 
+/// Sets a callback to be invoked on calls to write().  The callback is invoked
+/// before the write is done.  The write is not guaranteed to succeed when the
+/// callback executes.  Pass in NULL to remove any callback.
+void dfsan_set_write_callback(dfsan_write_callback_t labeled_write_callback);
+
 #ifdef __cplusplus
 }  // extern "C"
 
 template <typename T>
 void dfsan_set_label(dfsan_label label, T &data) {  // NOLINT
   dfsan_set_label(label, (void *)&data, sizeof(T));
 }
 
 #endif
 
 #endif  // DFSAN_INTERFACE_H

lib/dfsan/dfsan_custom.cc

__dfsw_socketpair(int domain, int type, int protocol, int sv[2],
                   dfsan_label *ret_label) {
   int ret = socketpair(domain, type, protocol, sv);
   *ret_label = 0;
   if (ret == 0) {
     dfsan_set_label(0, sv, sizeof(*sv) * 2);
   }
   return ret;
 }
+
+// Type of the trampoline function passed to the custom version of
+// dfsan_set_write_callback.
+typedef void (*write_trampoline_t)(
+    void *callback,
+    int fd, const void *buf, ssize_t count,
+    dfsan_label fd_label, dfsan_label buf_label, dfsan_label count_label);
+
+// Calls to dfsan_set_write_callback() set the values in this struct.
+// Calls to the custom version of write() read (and invoke) them.
+static struct {
+  write_trampoline_t write_callback_trampoline = NULL;
+  void *write_callback = NULL;
+} write_callback_info;
+
+SANITIZER_INTERFACE_ATTRIBUTE void
+__dfsw_dfsan_set_write_callback(
+    write_trampoline_t write_callback_trampoline,
+    void *write_callback,
+    dfsan_label write_callback_label,
+    dfsan_label *ret_label) {
+  write_callback_info.write_callback_trampoline = write_callback_trampoline;
+  write_callback_info.write_callback = write_callback;
+}
+
+SANITIZER_INTERFACE_ATTRIBUTE int
+__dfsw_write(int fd, const void *buf, size_t count,
+             dfsan_label fd_label, dfsan_label buf_label,
+             dfsan_label count_label, dfsan_label *ret_label) {
+  if (write_callback_info.write_callback != NULL) {
+    write_callback_info.write_callback_trampoline(
+        write_callback_info.write_callback,
+        fd, buf, count,
+        fd_label, buf_label, count_label);
+  }
+
+  *ret_label = 0;
+  return write(fd, buf, count);
+}
 }

lib/dfsan/done_abilist.txt

 fun:dfsan_get_label_count=uninstrumented
 fun:dfsan_get_label_count=discard
 fun:dfsan_get_label_info=uninstrumented
 fun:dfsan_get_label_info=discard
 fun:dfsan_has_label=uninstrumented
 fun:dfsan_has_label=discard
 fun:dfsan_has_label_with_desc=uninstrumented
 fun:dfsan_has_label_with_desc=discard
+fun:dfsan_set_write_callback=uninstrumented
+fun:dfsan_set_write_callback=custom
 
 ###############################################################################
 # glibc
 ###############################################################################
 fun:malloc=discard
 fun:realloc=discard
 fun:free=discard
 

 fun:socket=discard
 fun:strerror=discard
 fun:strspn=discard
 fun:strcspn=discard
 fun:symlink=discard
 fun:syscall=discard
 fun:unlink=discard
 fun:uselocale=discard
-fun:write=discard
 
 # Functions that produce output does not depend on the input (need to zero the
 # shadow manually).
 fun:calloc=custom
 fun:clock_gettime=custom
 fun:dlopen=custom
 fun:fgets=custom
 fun:fstat=custom

 fun:strchr=custom
 fun:strcmp=custom
 fun:strlen=custom
 fun:strncasecmp=custom
 fun:strncmp=custom
 fun:strrchr=custom
 fun:strstr=custom
 
+# Functions which take action based on global state, such as running a callback
+# set by a sepperate function.
+fun:write=custom
+
 # Functions that take a callback (wrap the callback manually).
 fun:dl_iterate_phdr=custom
 
 fun:getpwuid_r=custom
 fun:poll=custom
 fun:sched_getaffinity=custom
 fun:select=custom
 fun:sigemptyset=custom

test/dfsan/custom.c

 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
 #include <time.h>
 #include <unistd.h>
 
 dfsan_label i_label = 0;
 dfsan_label j_label = 0;
+dfsan_label k_label = 0;
 dfsan_label i_j_label = 0;
 
 #define ASSERT_ZERO_LABEL(data) \
   assert(0 == dfsan_get_label((long) (data)))
 
 #define ASSERT_READ_ZERO_LABEL(ptr, size) \
   assert(0 == dfsan_read_label(ptr, size))
 
void test_ctime_r() {
 
   t = 0;
   dfsan_set_label(j_label, &buf, sizeof(&buf));
   ret = ctime_r(&t, buf);
   ASSERT_LABEL(ret, j_label);
   ASSERT_READ_ZERO_LABEL(buf, strlen(buf) + 1);
 }
 
+static int write_callback_count = 0;
+static int last_fd;
+static const void *last_buf;
+static size_t last_count;
+
+void write_callback(int fd, const void *buf, size_t count) {
+  write_callback_count++;
+
+  last_fd = fd;
+  last_buf = buf;
+  last_count = count;
+}
+
+void test_dfsan_set_write_callback() {
+  char buf[] = "Sample chars";
+  int buf_len = strlen(buf);
+
+  int fd = open("/dev/null", O_WRONLY);
+
+  dfsan_set_write_callback(write_callback);
+
+  write_callback_count = 0;
+
+  // Callback should be invoked on every call to write().
+  int res = write(fd, buf, buf_len);
+  assert(write_callback_count == 1);
+  ASSERT_READ_ZERO_LABEL(&res, sizeof(res));
+  ASSERT_READ_ZERO_LABEL(&last_fd, sizeof(last_fd));
+  ASSERT_READ_ZERO_LABEL(last_buf, sizeof(last_buf));
+  ASSERT_READ_ZERO_LABEL(&last_count, sizeof(last_count));
+
+  // Add a label to write() arguments.  Check that the labels are readable from
+  // the values passed to the callback.
+  dfsan_set_label(i_label, &fd, sizeof(fd));
+  dfsan_set_label(j_label, &(buf[3]), 1);
+  dfsan_set_label(k_label, &buf_len, sizeof(buf_len));
+  
+  res = write(fd, buf, buf_len);
+  assert(write_callback_count == 2);
+  ASSERT_READ_ZERO_LABEL(&res, sizeof(res));
+  ASSERT_READ_LABEL(&last_fd, sizeof(last_fd), i_label);
+  ASSERT_READ_LABEL(&last_buf[3], sizeof(last_buf[3]), j_label);
+  ASSERT_READ_LABEL(last_buf, sizeof(last_buf), j_label);
+  ASSERT_READ_LABEL(&last_count, sizeof(last_count), k_label);
+
+  dfsan_set_write_callback(NULL);
+}
+
 void test_fgets() {
   char *buf = (char*) malloc(128);
   FILE *f = fopen("/etc/passwd", "r");
   dfsan_set_label(j_label, buf, 1);
   char *ret = fgets(buf, sizeof(buf), f);
   assert(ret == buf);
   ASSERT_ZERO_LABEL(ret);
   ASSERT_READ_ZERO_LABEL(buf, 128);
void test_socketpair() {
 
   dfsan_set_label(i_label, fd, sizeof(fd));
   int rv = socketpair(PF_LOCAL, SOCK_STREAM, 0, fd);
   assert(rv == 0);
   ASSERT_ZERO_LABEL(rv);
   ASSERT_READ_ZERO_LABEL(fd, sizeof(fd));
 }
 
+void test_write() {
+  int fd = open("/dev/null", O_WRONLY);
+
+  char buf[] = "a string";
+  int len = strlen(buf);
+
+  // The result of a write always unlabeled.
+  int res = write(fd, buf, len);
+  assert(res > 0);
+  ASSERT_ZERO_LABEL(res);
+
+  // Label all arguments to write().
+  dfsan_set_label(i_label, &(buf[3]), 1);
+  dfsan_set_label(j_label, &fd, sizeof(fd));
+  dfsan_set_label(i_label, &len, sizeof(len));
+
+  // The value returned by write() should have no label.
+  res = write(fd, buf, len);
+  ASSERT_ZERO_LABEL(res);
+
+  close(fd);
+}
+
 int main(void) {
   i_label = dfsan_create_label("i", 0);
   j_label = dfsan_create_label("j", 0);
+  k_label = dfsan_create_label("k", 0);
   i_j_label = dfsan_union(i_label, j_label);
 
   test_calloc();
   test_clock_gettime();
   test_ctime_r();
+  test_dfsan_set_write_callback();
   test_dl_iterate_phdr();
   test_dlopen();
   test_fgets();
   test_fstat();
   test_get_current_dir_name();
   test_getcwd();
   test_gethostname();
   test_getpwuid_r();
int main(void) {
   test_strrchr();
   test_strstr();
   test_strtod();
   test_strtol();
   test_strtoll();
   test_strtoul();
   test_strtoull();
   test_time();
+  test_write();
 }

test/dfsan/write_callback.c

+// RUN: %clang_dfsan                         -m64 %s -o %t && %t | FileCheck %s
+// RUN: %clang_dfsan  -mllvm -dfsan-args-abi -m64 %s -o %t && %t | FileCheck %s
+
+// Tests that the custom implementation of write() does writes with or without
+// a callback set using dfsan_set_write_callback().
+
+#include <sanitizer/dfsan_interface.h>
+
+#include <assert.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+// Check write callback arguments by having the callback store them in
+// the following variables:
+static int last_callback_arg_fd;
+static const void *last_callback_arg_buf;
+static size_t last_callback_arg_count;
+
+// Allow tests to check the number of callbacks made by incrementing
+// this count.  When callbacks are verified, the count is reset.
+static int count_unverified_callbacks = 0;
+
+// This callbact will be installed using dfsan_set_write_callback()
+// in tests below.
+static void write_callback(int fd, const void *buf, size_t count) {
+  // Do not do anything in this function that might call write().
+  count_unverified_callbacks++;
+
+  last_callback_arg_fd = fd;
+  last_callback_arg_buf = buf;
+  last_callback_arg_count = count;
+}
+
+static void write_string_to_stdout(char *string) {
+  char *cur = string;
+  int bytes_left = strlen(string);
+  while (bytes_left > 0) {
+    int res = write(fileno(stdout), cur, bytes_left);
+    assert (res >= 0);
+    cur += res;
+    bytes_left -= res;
+  }
+}
+
+static void test_can_write_without_callback() {
+  dfsan_set_write_callback(NULL);
+  count_unverified_callbacks = 0;
+
+  char aString[] = "Test that writes work without callback.\n";
+  // CHECK: Test that writes work without callback.
+  write_string_to_stdout(aString);
+
+  assert(count_unverified_callbacks == 0);
+}
+
+static void test_can_write_with_callback() {
+  dfsan_set_write_callback(write_callback);
+
+  count_unverified_callbacks = 0;
+
+  char stringWithCallback[] = "Test that writes work with callback.\n";
+  // CHECK: Test that writes work with callback.
+  write_string_to_stdout(stringWithCallback);
+
+  // Data was written, so at least one call to write() was made.
+  // Because a write may not process all the bytes it is passed, there
+  // may have been several calls to write().
+  assert(count_unverified_callbacks > 0);
+  count_unverified_callbacks = 0;
+
+  dfsan_set_write_callback(NULL);
+
+  char stringWithoutCallback[] = "Writes work after the callback is removed.\n";
+  // CHECK: Writes work after the callback is removed.
+  write_string_to_stdout(stringWithoutCallback);
+  assert(count_unverified_callbacks == 0);
+}
+
+static void test_failing_write_runs_callback() {
+  // Open /dev/null in read-only mode.  Calling write() on fd will fail.
+  int fd = open("/dev/null", O_RDONLY);
+  assert(fd != -1);
+
+  // Install a callback.
+  dfsan_set_write_callback(write_callback);
+
+  // Write to the read-only file handle.  The write will fail, but the callback
+  // should still be invoked.
+  char aString[] = "This text will fail to be written.\n";
+  int len = strlen(aString);
+  int write_result = write(fd, aString, len);
+  assert(write_result == -1);
+
+  assert(count_unverified_callbacks == 1);
+  count_unverified_callbacks = 0;
+
+  assert(fd == last_callback_arg_fd);
+  assert(aString == last_callback_arg_buf);
+  assert(len == last_callback_arg_count);
+
+  close(fd);
+}
+
+int main(int argc, char* argv[]) {
+  test_can_write_without_callback();
+  test_can_write_with_callback();
+  test_failing_write_runs_callback();
+}