[PATCH v2] Ensure /dev/mem is a character device file

[Jean Delvare]

While option --dev-mem can be convenient for testing purposes, it
could be abused by attackers to force dmidecode to read a malicious
file. Add a safety check on the type of the mem device file we are
asked to read from. If we are root and this isn't a character device
file, then something is fishy and we better stop.

For non-root users, reading from a regular file is OK and accepted.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
---
Changes since v1:
 * Moved the check to function mem_chunk() to close a race condition
   noticed by my colleague Matthias Gerstner. A nice side effect is
   that the check also covers the other utilities (biosdecode etc.)
   now.
 * Don't provide detailed information on failure, so as to not help
   attackers.

 util.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

--- dmidecode.orig/util.c
+++ dmidecode/util.c
@@ -173,18 +173,26 @@ static void safe_memcpy(void *dest, cons
  */
 void *mem_chunk(off_t base, size_t len, const char *devmem)
 {
-       void *p;
+       struct stat statbuf;
+       void *p = NULL;
        int fd;
 #ifdef USE_MMAP
-       struct stat statbuf;
        off_t mmoffset;
        void *mmp;
 #endif
 
-       if ((fd = open(devmem, O_RDONLY)) == -1)
+       /*
+        * Safety check: if running as root, devmem is expected to be a
+        * character device file.
+        */
+       if ((fd = open(devmem, O_RDONLY)) == -1
+        || fstat(fd, &statbuf) == -1
+        || (geteuid() == 0 && !S_ISCHR(statbuf.st_mode)))
        {
-               perror(devmem);
-               return NULL;
+               fprintf(stderr, "Can't read memory from %s\n", devmem);
+               if (fd == -1)
+                       return NULL;
+               goto out;
        }
 
        if ((p = malloc(len)) == NULL)
@@ -194,13 +202,6 @@ void *mem_chunk(off_t base, size_t len,
        }
 
 #ifdef USE_MMAP
-       if (fstat(fd, &statbuf) == -1)
-       {
-               fprintf(stderr, "%s: ", devmem);
-               perror("stat");
-               goto err_free;
-       }
-
        /*
         * mmap() will fail with SIGBUS if trying to map beyond the end of
         * the file.


-- 
Jean Delvare
SUSE L3 Support