[Toybox] [PATCH] devmem: Fix 8 byte wide writes

[Michael Shavit]

Replace the left-shift method of computing the maximum allowed write as
it may exibit undefined behavior.
Add and use an unsigned version of atolx to support writing any 64bit
value.

* When bytes equals 8, devmem will left-shift an unsigned long long by
  64 bits which is undefined behavior if sizeof(unsigned long long) is
  64.
* In addition, atolx_range's 'high' parameter is a signed long long and
  will therefore interpret a high value of ULLONG_MAX as -1.
* Finally, atolx relies on strtoll under the hood, which will reject
  values beyond LLONG_MAX.

---

 lib/lib.c           | 39 +++++++++++++++++++++++++++++++++++++++
 lib/lib.h           |  1 +
 toys/other/devmem.c | 17 +++++++++++++----
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/lib/lib.c b/lib/lib.c
index c4e70dfe..315f4ef5 100644
--- a/lib/lib.c
+++ b/lib/lib.c
@@ -335,6 +335,45 @@ long long atolx_range(char *numstr, long long low, long long high)
   return val;
 }
 
+static unsigned long long estrtoull(char *str, char **end, int base)
+{
+  errno = 0;
+
+  return strtoull(str, end, base);
+}
+
+static unsigned long long xstrtoull(char *str, char **end, int base)
+{
+  unsigned long long l = estrtoull(str, end, base);
+
+  if (errno) perror_exit_raw(str);
+
+  return l;
+}
+
+// Unsigned version of atolx()
+unsigned long long atoullx(char *numstr)
+{
+  char *c = numstr, *suffixes="cwbkmgtpe", *end;
+  unsigned long long val;
+
+  val = xstrtoull(numstr, &c, 0);
+  if (c != numstr && *c && (end = strchr(suffixes, tolower(*c)))) {
+    int shift = end-suffixes-2;
+    ++c;
+    if (shift==-1) val *= 2;
+    else if (!shift) val *= 512;
+    else if (shift>0) {
+      if (*c && tolower(*c++)=='d') while (shift--) val *= 1000;
+      else val *= 1LL<<(shift*10);
+    }
+  }
+  while (isspace(*c)) c++;
+  if (c==numstr || *c) error_exit("not integer: %s", numstr);
+
+  return val;
+}
+
 int stridx(char *haystack, char needle)
 {
   char *off;
diff --git a/lib/lib.h b/lib/lib.h
index 6851c4aa..0c3a7d8a 100644
--- a/lib/lib.h
+++ b/lib/lib.h
@@ -226,6 +226,7 @@ void poke(void *ptr, long long val, unsigned size);
 struct string_list *find_in_path(char *path, char *filename);
 long long estrtol(char *str, char **end, int base);
 long long xstrtol(char *str, char **end, int base);
+unsigned long long atoullx(char *c);
 long long atolx(char *c);
 long long atolx_range(char *numstr, long long low, long long high);
 int stridx(char *haystack, char needle);
diff --git a/toys/other/devmem.c b/toys/other/devmem.c
index ced6c518..7c34b32f 100644
--- a/toys/other/devmem.c
+++ b/toys/other/devmem.c
@@ -21,9 +21,9 @@ config DEVMEM
 void devmem_main(void)
 {
   int writing = toys.optc == 3, page_size = sysconf(_SC_PAGESIZE), bytes = 4,fd;
-  unsigned long long addr = atolx(toys.optargs[0]), data = 0, map_off, map_len;
+  unsigned long long addr = atoullx(toys.optargs[0]), data = 0, map_off, map_len;
+  unsigned long long max_value = ~0ULL;
   void *map, *p;
-
   // WIDTH?
   if (toys.optc>1) {
     int i;
@@ -33,8 +33,17 @@ void devmem_main(void)
     bytes = 1<<i;
   }
 
-  // DATA? Report out of range values as errors rather than truncating.
-  if (writing) data = atolx_range(toys.optargs[2], 0, (1ULL<<(8*bytes))-1);
+  if (writing) {
+    // Shifting 1 left by N (and then decrementing by 1) to compute the maximum
+    // value that can fit in N bits can result in undefined behavior if N is
+    // equal to the bit length of the operand. So instead, right shift the
+    // maximum value that fits in data such that only the right-most `width`
+    // bytes are set.
+    max_value = max_value >> (8 * (sizeof(max_value) - bytes));
+    data = atoullx(toys.optargs[2]);
+    if (data > max_value)
+      error_exit("data: %llu exceeds write width: %d", data, bytes);
+  }
 
   // Map in just enough.
   fd = xopen("/dev/mem", (writing ? O_RDWR : O_RDONLY) | O_SYNC);

base-commit: 67a494e201216fb9eaa8ce685e57656036c75081
-- 
2.42.0.rc2.253.gd59a3bf2b4-goog