commit 6a095dfa6bd2000f650308cdfa5a69e2635c02ec
author Lorenzo Colitti <lorenzo@google.com> Wed Apr 10 23:22:30 2019 +0900
committer Lorenzo Colitti <lorenzo@google.com> Wed Apr 10 23:50:39 2019 +0900
tree 0a8afeddec201ee6f1b47629cd6bdaefad13b8ca
parent 60bce378cfa29463f57756fc724d1572747f4fc5

Enable clang-tidy for clatd.

Enable the same warnings used elsewhere in the tree, and fix two
warnings it found (a safe use of strcpy, and a missing O_CLOEXEC
when opening the tun device node.

Test: builds, boots, clatd works
Test: m clatd clatd_test clatd_microbenchmark && atest clatd_test
Change-Id: I9a5ea4de5f31d3c495871250a6493b07535a604b

Android.bp

diff --git a/Android.bp b/Android.bp
index e0eeb2e..98b4010 100644
--- a/Android.bp
+++ b/Android.bp
@@ -54,6 +54,20 @@
         "liblog",
         "libnetutils",
     ],
+
+    // Only enable clang-tidy for the daemon, not the tests, because enabling it for the
+    // tests substantially increases build/compile cycle times and doesn't really provide a
+    // security benefit.
+    tidy: true,
+    tidy_checks: [
+        "-*",
+        "cert-*",
+        "clang-analyzer-security*",
+        "android-*",
+    ],
+    tidy_flags: [
+        "-warnings-as-errors=clang-analyzer-security*,cert-*,android-*",
+    ],
 }
 
 // The configuration file.

clatd.c

diff --git a/clatd.c b/clatd.c
index 06ca799..d68dc05 100644
--- a/clatd.c
+++ b/clatd.c
@@ -381,7 +381,7 @@
     logmsg(ANDROID_LOG_WARN, "ipv4mtu now set to = %d", Global_Clatd_Config.ipv4mtu);
   }
 
-  error = tun_alloc(tunnel->device4, tunnel->fd4);
+  error = tun_alloc(tunnel->device4, tunnel->fd4, sizeof(tunnel->device4));
   if (error < 0) {
     logmsg(ANDROID_LOG_FATAL, "tun_alloc/4 failed: %s", strerror(errno));
     exit(1);

clatd_microbenchmark.c

diff --git a/clatd_microbenchmark.c b/clatd_microbenchmark.c
index 91b0996..15a0376 100644
--- a/clatd_microbenchmark.c
+++ b/clatd_microbenchmark.c
@@ -67,7 +67,7 @@
   if (fd == -1) die("tun_open");
 
   char dev[IFNAMSIZ] = DEVICENAME;
-  int ret            = tun_alloc(dev, fd);
+  int ret            = tun_alloc(dev, fd, sizeof(dev));
   if (ret == -1) die("tun_alloc");
   struct ifreq ifr = {
     .ifr_name = DEVICENAME,

tun.c

diff --git a/tun.c b/tun.c
index 406fc2f..7ecbf2c 100644
--- a/tun.c
+++ b/tun.c
@@ -32,9 +32,9 @@
 int tun_open() {
   int fd;
 
-  fd = open("/dev/tun", O_RDWR);
+  fd = open("/dev/tun", O_RDWR | O_CLOEXEC);
   if (fd < 0) {
-    fd = open("/dev/net/tun", O_RDWR);
+    fd = open("/dev/net/tun", O_RDWR | O_CLOEXEC);
   }
 
   return fd;
@@ -43,8 +43,10 @@
 /* function: tun_alloc
  * creates a tun interface and names it
  * dev - the name for the new tun device
+ * fd - an open fd to the tun device node
+ * len - the length of the buffer pointed to by dev
  */
-int tun_alloc(char *dev, int fd) {
+int tun_alloc(char *dev, int fd, size_t len) {
   struct ifreq ifr;
   int err;
 
@@ -60,7 +62,7 @@
     close(fd);
     return err;
   }
-  strcpy(dev, ifr.ifr_name);
+  strlcpy(dev, ifr.ifr_name, len);
   return 0;
 }
 

tun.h

diff --git a/tun.h b/tun.h
index f0449b9..95650fa 100644
--- a/tun.h
+++ b/tun.h
@@ -30,7 +30,7 @@
 };
 
 int tun_open();
-int tun_alloc(char *dev, int fd);
+int tun_alloc(char *dev, int fd, size_t len);
 int send_tun(int fd, clat_packet out, int iov_len);
 int set_nonblocking(int fd);