Everyone forgets that mmap returns MAP_FAILED rather than NULL on failure.
Every use of mmap in toybox was either doing the wrong check, or no check
at all (including the two I personally added).
---
 lib/lib.h                  |   1 +
 lib/xwrap.c                |   7 ++
 toys/android/load_policy.c |   6 +-
 toys/other/hexedit.c       |   2 +-
 toys/other/modinfo.c       |   4 +-
 toys/pending/mdev.c        | 175 ++++++++++++++++++++++-----------------------
 toys/posix/file.c          |   3 +-
 7 files changed, 102 insertions(+), 96 deletions(-)
From f8034afa1259d8853ea9dcb6dfbf5a74566ca01c Mon Sep 17 00:00:00 2001
From: Elliott Hughes <e...@google.com>
Date: Tue, 23 May 2017 17:35:49 -0700
Subject: [PATCH] Add and use xmmap.

Everyone forgets that mmap returns MAP_FAILED rather than NULL on failure.
Every use of mmap in toybox was either doing the wrong check, or no check
at all (including the two I personally added).
---
 lib/lib.h                  |   1 +
 lib/xwrap.c                |   7 ++
 toys/android/load_policy.c |   6 +-
 toys/other/hexedit.c       |   2 +-
 toys/other/modinfo.c       |   4 +-
 toys/pending/mdev.c        | 175 ++++++++++++++++++++++-----------------------
 toys/posix/file.c          |   3 +-
 7 files changed, 102 insertions(+), 96 deletions(-)

diff --git a/lib/lib.h b/lib/lib.h
index 5d2bb4d..5e3ad2b 100644
--- a/lib/lib.h
+++ b/lib/lib.h
@@ -115,6 +115,7 @@ void xstrncpy(char *dest, char *src, size_t size);
 void xstrncat(char *dest, char *src, size_t size);
 void _xexit(void) noreturn;
 void xexit(void) noreturn;
+void *xmmap(void *addr, size_t length, int prot, int flags, int fd, off_t off);
 void *xmalloc(size_t size);
 void *xzalloc(size_t size);
 void *xrealloc(void *ptr, size_t size);
diff --git a/lib/xwrap.c b/lib/xwrap.c
index 0281e70..83f54a0 100644
--- a/lib/xwrap.c
+++ b/lib/xwrap.c
@@ -58,6 +58,13 @@ void xexit(void)
   _xexit();
 }
 
+void *xmmap(void *addr, size_t length, int prot, int flags, int fd, off_t off)
+{
+  void *ret = mmap(addr, length, prot, flags, fd, off);
+  if (ret == MAP_FAILED) perror_exit("mmap");
+  return ret;
+}
+
 // Die unless we can allocate memory.
 void *xmalloc(size_t size)
 {
diff --git a/toys/android/load_policy.c b/toys/android/load_policy.c
index d9ff148..bc0d564 100644
--- a/toys/android/load_policy.c
+++ b/toys/android/load_policy.c
@@ -21,11 +21,11 @@ void load_policy_main(void)
 {
   int fd = xopenro(*toys.optargs);
   off_t policy_len = fdlength(fd);
-  char *policy_data = mmap(0, policy_len, PROT_READ, MAP_PRIVATE, fd, 0);
+  char *policy_data = xmmap(0, policy_len, PROT_READ, MAP_PRIVATE, fd, 0);
 
   close(fd);
-  if (!policy_data || security_load_policy(policy_data, policy_len) < 0)
-    perror_exit("Couldn't %s %s", policy_data ? "load" : "read", *toys.optargs);
+  if (security_load_policy(policy_data, policy_len) < 0)
+    perror_exit("security_load_policy %s", *toys.optargs);
 
   munmap(policy_data, policy_len);
 }
diff --git a/toys/other/hexedit.c b/toys/other/hexedit.c
index ffa304c..3c5ada3 100644
--- a/toys/other/hexedit.c
+++ b/toys/other/hexedit.c
@@ -140,7 +140,7 @@ void hexedit_main(void)
   for (pos = TT.len, TT.numlen = 0; pos; pos >>= 4, TT.numlen++);
   TT.numlen += (4-TT.numlen)&3;
 
-  TT.data = mmap(0, TT.len, PROT_READ|(PROT_WRITE*!ro), MAP_SHARED, fd, 0);
+  TT.data = xmmap(0, TT.len, PROT_READ|(PROT_WRITE*!ro), MAP_SHARED, fd, 0);
   draw_page();
 
   for (;;) {
diff --git a/toys/other/modinfo.c b/toys/other/modinfo.c
index 69cdf27..1178d67 100644
--- a/toys/other/modinfo.c
+++ b/toys/other/modinfo.c
@@ -46,7 +46,8 @@ static void modinfo_file(char *full_name)
 
   if (-1 != (fd = open(full_name, O_RDONLY))) {
     len = fdlength(fd);
-    if (!(buf = mmap(0, len, PROT_READ, MAP_SHARED, fd, 0))) close(fd);
+    buf = xmmap(0, len, PROT_READ, MAP_SHARED, fd, 0);
+    close(fd);
   }
 
   if (!buf) {
@@ -69,7 +70,6 @@ static void modinfo_file(char *full_name)
   }
 
   munmap(buf, len);
-  close(fd);
 }
 
 static int check_module(struct dirtree *new)
diff --git a/toys/pending/mdev.c b/toys/pending/mdev.c
index 2688cf3..1cf4e7c 100644
--- a/toys/pending/mdev.c
+++ b/toys/pending/mdev.c
@@ -82,103 +82,102 @@ static void make_device(char *path)
 
     // mmap the config file
     if (-1!=(fd = open("/etc/mdev.conf", O_RDONLY))) {
+      int line = 0;
+
       len = fdlength(fd);
-      conf = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fd, 0);
-      if (conf) {
-        int line = 0;
-
-        // Loop through lines in mmaped file
-        for (pos = conf; pos-conf<len;) {
-          int field;
-          char *end2;
-
-          line++;
-          // find end of this line
-          for(end = pos; end-conf<len && *end!='\n'; end++);
-
-          // Three fields: regex, uid:gid, mode
-          for (field = 3; field; field--) {
-            // Skip whitespace
-            while (pos<end && isspace(*pos)) pos++;
-            if (pos==end || *pos=='#') break;
-            for (end2 = pos;
-              end2<end && !isspace(*end2) && *end2!='#'; end2++);
-            switch(field) {
-              // Regex to match this device
-              case 3:
-              {
-                char *regex = strndup(pos, end2-pos);
-                regex_t match;
-                regmatch_t off;
-                int result;
-
-                // Is this it?
-                xregcomp(&match, regex, REG_EXTENDED);
-                result=regexec(&match, device_name, 1, &off, 0);
-                regfree(&match);
-                free(regex);
-
-                // If not this device, skip rest of line
-                if (result || off.rm_so
-                  || off.rm_eo!=strlen(device_name))
-                    goto end_line;
-
-                break;
-              }
-              // uid:gid
-              case 2:
-              {
-                char *s2;
-
-                // Find :
-                for(s = pos; s<end2 && *s!=':'; s++);
-                if (s==end2) goto end_line;
-
-                // Parse UID
-                uid = strtoul(pos,&s2,10);
-                if (s!=s2) {
-                  struct passwd *pass;
-                  char *str = strndup(pos, s-pos);
-                  pass = getpwnam(str);
-                  free(str);
-                  if (!pass) goto end_line;
-                  uid = pass->pw_uid;
-                }
-                s++;
-                // parse GID
-                gid = strtoul(s,&s2,10);
-                if (end2!=s2) {
-                  struct group *grp;
-                  char *str = strndup(s, end2-s);
-                  grp = getgrnam(str);
-                  free(str);
-                  if (!grp) goto end_line;
-                  gid = grp->gr_gid;
-                }
-                break;
+      conf = xmmap(NULL, len, PROT_READ, MAP_PRIVATE, fd, 0);
+
+      // Loop through lines in mmaped file
+      for (pos = conf; pos-conf<len;) {
+        int field;
+        char *end2;
+
+        line++;
+        // find end of this line
+        for(end = pos; end-conf<len && *end!='\n'; end++);
+
+        // Three fields: regex, uid:gid, mode
+        for (field = 3; field; field--) {
+          // Skip whitespace
+          while (pos<end && isspace(*pos)) pos++;
+          if (pos==end || *pos=='#') break;
+          for (end2 = pos;
+            end2<end && !isspace(*end2) && *end2!='#'; end2++);
+          switch(field) {
+            // Regex to match this device
+            case 3:
+            {
+              char *regex = strndup(pos, end2-pos);
+              regex_t match;
+              regmatch_t off;
+              int result;
+
+              // Is this it?
+              xregcomp(&match, regex, REG_EXTENDED);
+              result=regexec(&match, device_name, 1, &off, 0);
+              regfree(&match);
+              free(regex);
+
+              // If not this device, skip rest of line
+              if (result || off.rm_so
+                || off.rm_eo!=strlen(device_name))
+                  goto end_line;
+
+              break;
+            }
+            // uid:gid
+            case 2:
+            {
+              char *s2;
+
+              // Find :
+              for(s = pos; s<end2 && *s!=':'; s++);
+              if (s==end2) goto end_line;
+
+              // Parse UID
+              uid = strtoul(pos,&s2,10);
+              if (s!=s2) {
+                struct passwd *pass;
+                char *str = strndup(pos, s-pos);
+                pass = getpwnam(str);
+                free(str);
+                if (!pass) goto end_line;
+                uid = pass->pw_uid;
               }
-              // mode
-              case 1:
-              {
-                mode = strtoul(pos, &pos, 8);
-                if (pos!=end2) goto end_line;
-                goto found_device;
+              s++;
+              // parse GID
+              gid = strtoul(s,&s2,10);
+              if (end2!=s2) {
+                struct group *grp;
+                char *str = strndup(s, end2-s);
+                grp = getgrnam(str);
+                free(str);
+                if (!grp) goto end_line;
+                gid = grp->gr_gid;
               }
+              break;
+            }
+            // mode
+            case 1:
+            {
+              mode = strtoul(pos, &pos, 8);
+              if (pos!=end2) goto end_line;
+              goto found_device;
             }
-            pos=end2;
           }
+          pos=end2;
+        }
 end_line:
-          // Did everything parse happily?
-          if (field && field!=3) error_exit("Bad line %d", line);
+        // Did everything parse happily?
+        if (field && field!=3) error_exit("Bad line %d", line);
 
-          // Next line
-          pos = ++end;
-        }
-found_device:
-        munmap(conf, len);
+        // Next line
+        pos = ++end;
       }
-      close(fd);
+found_device:
+      munmap(conf, len);
     }
+    close(fd);
   }
 
   sprintf(toybuf, "/dev/%s", device_name);
diff --git a/toys/posix/file.c b/toys/posix/file.c
index 9fa8ed0..658708c 100644
--- a/toys/posix/file.c
+++ b/toys/posix/file.c
@@ -109,8 +109,7 @@ static void do_elf_file(int fd, struct stat *sb)
     return;
   }
 
-  map = mmap(0, sb->st_size, PROT_READ, MAP_SHARED, fd, 0);
-  if (!map) perror_exit("mmap");
+  map = xmmap(0, sb->st_size, PROT_READ, MAP_SHARED, fd, 0);
 
   // We need to read the phdrs for dynamic vs static.
   // (Note: fields got reordered for 64 bit)
-- 
2.13.0.219.gdb65acc882-goog

_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to