fixup! [Pal,LibOS] Refactor sysfs to improve sanitization

LibOS/shim/include/shim_fs_pseudo.h

@@ -209,18 +209,18 @@ int sys_cache_load(struct shim_dentry* dent, char** out_data, size_t* out_size);
 bool sys_cpu_online_name_exists(struct shim_dentry* parent, const char* name);
 int sys_cpu_online_list_names(struct shim_dentry* parent, readdir_callback_t callback, void* arg);
 
-/* Converts integers to a string. If size multiplier like 'K', 'M' or 'G' is passed, it is appended
+/* Converts integer to a string. If size multiplier like 'K', 'M' or 'G' is passed, it is appended
  * to the string. */
-int sys_convert_int_to_str(uint64_t val, uint64_t size_mult, char* str, int max_len);
+int sys_convert_int_to_str(uint64_t val, uint64_t size_mult, char* str, size_t max_len);
 
-/* Converts integer ranges to a string. For example if ranges.start and ranges.end were 0 and 63
- * respectively, then `0-63` string is generated. */
-int sys_convert_range_to_str(const PAL_RES_RANGE_INFO* res_range_info, char* str, int max_len,
-                             const char* sep);
+/* Converts array of integer range(s) to a string. For example if ranges[0].start and ranges[0].end
+ * were 0 and 63 respectively, then `0-63` string is generated. */
+int sys_convert_ranges_to_str(const PAL_RES_RANGE_INFO* res_range_info, char* str, size_t max_len,
+                              const char* sep);
 
-/* Converts integer ranges to a cpu bitmap string. For example, if ranges.start and ranges.end were
- * 32 and 63, then `ffffffff,00000000` string is generated. */
-int sys_convert_range_to_cpu_bitmap_str(const PAL_RES_RANGE_INFO* res_range_info, char* str,
-                                        int max_len);
+/* Converts array of integer range(s) to a cpu bitmap string. For example, if ranges[0].start and
+ * ranges[0].end were 0 and 31 respectively, then `00000000,ffffffff` string is generated. */
+int sys_convert_ranges_to_cpu_bitmap_str(const PAL_RES_RANGE_INFO* res_range_info, char* str,
+                                         size_t max_len);
 
 #endif /* SHIM_FS_PSEUDO_H_ */

LibOS/shim/src/fs/sys/cache_info.c

@@ -29,18 +29,18 @@ int sys_cache_load(struct shim_dentry* dent, char** out_data, size_t* out_size)
     PAL_CORE_CACHE_INFO* cache = &g_pal_control->topo_info.core_topology[cpu_num].cache[cache_num];
     char str[PAL_SYSFS_MAP_FILESZ] = {'\0'};
     if (strcmp(name, "shared_cpu_map") == 0) {
-        ret = sys_convert_range_to_cpu_bitmap_str(&cache->shared_cpu_map, str, sizeof(str));
+        ret = sys_convert_ranges_to_cpu_bitmap_str(&cache->shared_cpu_map, str, sizeof(str));
     } else if (strcmp(name, "level") == 0) {
         ret = sys_convert_int_to_str(cache->level, MULTIPLIER_NONE, str, sizeof(str));
     } else if (strcmp(name, "type") == 0) {
         switch (cache->type) {
-            case DATA:
+            case CACHE_TYPE_DATA:
                 ret = snprintf(str, sizeof(str), "%s", "Data\n");
                 break;
-            case INSTRUCTION:
+            case CACHE_TYPE_INSTRUCTION:
                 ret = snprintf(str, sizeof(str), "%s", "Instruction\n");
                 break;
-            case UNIFIED:
+            case CACHE_TYPE_UNIFIED:
                 ret = snprintf(str, sizeof(str), "%s", "Unified\n");
                 break;
             default:

LibOS/shim/src/fs/sys/cpu_info.c

@@ -13,16 +13,16 @@
 #include "shim_fs_pseudo.h"
 
 int sys_cpu_general_load(struct shim_dentry* dent, char** out_data, size_t* out_size) {
-    int ret = 0;
+    int ret;
     const char* name = dent->name;
     char str[PAL_SYSFS_BUF_FILESZ] = {'\0'};
 
     if (strcmp(name, "online") == 0) {
-        ret = sys_convert_range_to_str(&g_pal_control->topo_info.online_logical_cores, str,
-                                       sizeof(str), ",");
+        ret = sys_convert_ranges_to_str(&g_pal_control->topo_info.online_logical_cores, str,
+                                        sizeof(str), ",");
     } else if (strcmp(name, "possible") == 0) {
-        ret = sys_convert_range_to_str(&g_pal_control->topo_info.possible_logical_cores, str,
-                                       sizeof(str), ",");
+        ret = sys_convert_ranges_to_str(&g_pal_control->topo_info.possible_logical_cores, str,
+                                        sizeof(str), ",");
     } else {
         log_debug("unrecognized file: %s", name);
         ret = -ENOENT;
@@ -54,10 +54,10 @@ int sys_cpu_load(struct shim_dentry* dent, char** out_data, size_t* out_size) {
     } else if (strcmp(name, "physical_package_id") == 0) {
         ret = sys_convert_int_to_str(core_topology->cpu_socket, MULTIPLIER_NONE, str, sizeof(str));
     } else if (strcmp(name, "core_siblings") == 0) {
-        ret = sys_convert_range_to_cpu_bitmap_str(&core_topology->core_siblings, str, sizeof(str));
+        ret = sys_convert_ranges_to_cpu_bitmap_str(&core_topology->core_siblings, str, sizeof(str));
     } else if (strcmp(name, "thread_siblings") == 0) {
-        ret = sys_convert_range_to_cpu_bitmap_str(&core_topology->thread_siblings, str,
-                                                  sizeof(str));
+        ret = sys_convert_ranges_to_cpu_bitmap_str(&core_topology->thread_siblings, str,
+                                                   sizeof(str));
     } else {
         log_debug("unrecognized file: %s", name);
         ret = -ENOENT;

LibOS/shim/src/fs/sys/fs.c

@@ -14,8 +14,10 @@
 #include "shim_fs_pseudo.h"
 #include "stat.h"
 
-int sys_convert_int_to_str(uint64_t val, uint64_t size_mult, char* str, int max_len) {
+int sys_convert_int_to_str(uint64_t val, uint64_t size_mult, char* str, size_t max_len) {
     int ret = 0;
+    assert(max_len > 0);
+
     switch (size_mult) {
         case MULTIPLIER_KB:
             ret = snprintf(str, max_len, "%luK", val);
@@ -33,47 +35,45 @@ int sys_convert_int_to_str(uint64_t val, uint64_t size_mult, char* str, int max_
     return ret;
 }
 
-int sys_convert_range_to_str(const PAL_RES_RANGE_INFO* res_range_info, char* str, int max_len,
-                             const char* sep) {
+int sys_convert_ranges_to_str(const PAL_RES_RANGE_INFO* res_range_info, char* str, size_t max_len,
+                              const char* sep) {
     if (res_range_info->range_count > INT64_MAX)
         return -EINVAL;
 
     int64_t range_cnt = (int64_t)res_range_info->range_count;
-    int offset = 0;
+    size_t offset = 0;
     for (int64_t i = 0; i < range_cnt; i++) {
-        if (max_len - offset < 0)
+        if (offset > max_len)
             return -ENOMEM;
 
         int ret;
-        char end_str[PAL_SYSFS_BUF_FILESZ] = {'\0'};
-        if (res_range_info->ranges[i].end == UINT64_MAX) {
-            ret = snprintf(end_str, sizeof(end_str), "%s", "");
+        if (res_range_info->ranges[i].end == res_range_info->ranges[i].start) {
+            ret = snprintf(str + offset, max_len - offset, "%lu%s", res_range_info->ranges[i].start,
+                           (i + 1 == range_cnt) ? "\0" : sep);
         } else {
-            ret = snprintf(end_str, sizeof(end_str), "-%lu", res_range_info->ranges[i].end);
+            ret = snprintf(str + offset, max_len - offset, "%lu-%lu%s",
+                           res_range_info->ranges[i].start, res_range_info->ranges[i].end,
+                           (i + 1 == range_cnt) ? "\0" : sep);
         }
 
         if (ret < 0)
             return ret;
 
-        ret = snprintf(str + offset, max_len - offset, "%lu%s%s", res_range_info->ranges[i].start,
-                       end_str, (i + 1 == range_cnt) ? "\0" : sep);
-        if (ret < 0)
-            return ret;
         offset += ret;
     }
     return 0;
 }
 
-int sys_convert_range_to_cpu_bitmap_str(const PAL_RES_RANGE_INFO* res_range_info, char* str,
-                                        int max_len) {
+int sys_convert_ranges_to_cpu_bitmap_str(const PAL_RES_RANGE_INFO* res_range_info, char* str,
+                                         size_t max_len) {
     if (g_pal_control->topo_info.possible_logical_cores.resource_count > INT64_MAX)
         return -1;
-    int ret = 0;
+    int ret;
 
     /* Extract cpumask from the ranges */
     int64_t possible_cores =  g_pal_control->topo_info.possible_logical_cores.resource_count;
     int64_t num_cpumask = BITS_TO_INTS(possible_cores);
-    unsigned int* bitmap = (unsigned int*)calloc(num_cpumask, sizeof(unsigned int));
+    uint32_t* bitmap = (uint32_t*)calloc(num_cpumask, sizeof(uint32_t));
     if (!bitmap)
         return -ENOMEM;
 
@@ -83,8 +83,6 @@ int sys_convert_range_to_cpu_bitmap_str(const PAL_RES_RANGE_INFO* res_range_info
     for (int64_t i = 0; i < (int64_t)res_range_info->range_count; i++) {
         uint64_t start = res_range_info->ranges[i].start;
         uint64_t end = res_range_info->ranges[i].end;
-        if (end == UINT64_MAX)
-            end = start;
         if (start > INT64_MAX || end > INT64_MAX)
             return -EINVAL;
         for (int64_t j = (int64_t)start; j <= (int64_t)end; j++) {
@@ -98,9 +96,9 @@ int sys_convert_range_to_cpu_bitmap_str(const PAL_RES_RANGE_INFO* res_range_info
     }
 
     /* Convert cpumask to strings */
-    int offset = 0;
+    size_t offset = 0;
     for (int64_t j = num_cpumask - 1; j >= 0; j-- ) {
-        if (max_len - offset < 0) {
+        if (offset > max_len) {
             ret = -ENOMEM;
             goto out;
         }

LibOS/shim/src/fs/sys/node_info.c

@@ -12,11 +12,12 @@
 #include "shim_fs_pseudo.h"
 
 int sys_node_general_load(struct shim_dentry* dent, char** out_data, size_t* out_size) {
+    int ret;
+
     const char* name = dent->name;
-    char str[PAL_SYSFS_BUF_FILESZ] = {'\0'};
-    int ret = 0;
+    char str[PAL_SYSFS_BUF_FILESZ] = {'\0'};;
     if (strcmp(name, "online") == 0) {
-        ret = sys_convert_range_to_str(&g_pal_control->topo_info.nodes, str, sizeof(str), ",");
+        ret = sys_convert_ranges_to_str(&g_pal_control->topo_info.nodes, str, sizeof(str), ",");
     } else {
         log_debug("unrecognized file: %s", name);
         ret = -ENOENT;
@@ -38,9 +39,9 @@ int sys_node_load(struct shim_dentry* dent, char** out_data, size_t* out_size) {
     PAL_NUMA_TOPO_INFO* numa_topology = &g_pal_control->topo_info.numa_topology[node_num];
     char str[PAL_SYSFS_MAP_FILESZ] = {'\0'};
     if (strcmp(name, "cpumap" ) == 0) {
-        ret = sys_convert_range_to_cpu_bitmap_str(&numa_topology->cpumap, str, sizeof(str));
+        ret = sys_convert_ranges_to_cpu_bitmap_str(&numa_topology->cpumap, str, sizeof(str));
     } else if (strcmp(name, "distance") == 0) {
-        ret = sys_convert_range_to_str(&numa_topology->distance, str, sizeof(str), " ");
+        ret = sys_convert_ranges_to_str(&numa_topology->distance, str, sizeof(str), " ");
     } else if (strcmp(name, "nr_hugepages") == 0) {
         const char* parent_name = dent->parent->name;
         if (strcmp(parent_name, "hugepages-2048kB") == 0) {
@@ -49,6 +50,8 @@ int sys_node_load(struct shim_dentry* dent, char** out_data, size_t* out_size) {
         } else if (strcmp(parent_name, "hugepages-1048576kB") == 0) {
             ret = sys_convert_int_to_str(numa_topology->nr_hugepages[HUGEPAGES_1G], MULTIPLIER_NONE,
                                          str, sizeof(str));
+        } else {
+            log_debug("unrecognized hugepage file: %s", parent_name);
         }
     } else {
         log_debug("unrecognized file: %s", name);

Pal/include/arch/x86_64/pal-arch.h

@@ -31,12 +31,10 @@ typedef struct pal_tcb PAL_TCB;
 #define STACK_PROTECTOR_CANARY_DEFAULT  0xbadbadbadbadUL
 
 /* Used to represent buffers having numeric values with text. E.g "1024576K".
- * Note: This MACRO is used to allocate on the stack, so increase the size if needed with caution or
- * use malloc. */
+ * NOTE: Used to allocate on stack; increase with caution or use malloc instead. */
 #define PAL_SYSFS_BUF_FILESZ 64
 /* Used to represent cpumaps like "00000000,ffffffff,00000000,ffffffff".
- * Note: This MACRO is used to allocate on the stack, so increase the size if needed with caution or
- * use malloc. */
+ * NOTE: Used to allocate on stack; increase with caution or use malloc instead. */
 #define PAL_SYSFS_MAP_FILESZ 256
 
 typedef struct pal_tcb {
@@ -288,9 +286,9 @@ enum {
 };
 
 enum {
-    DATA = 0,
-    INSTRUCTION,
-    UNIFIED,
+    CACHE_TYPE_DATA = 0,
+    CACHE_TYPE_INSTRUCTION,
+    CACHE_TYPE_UNIFIED,
 };
 
 /* PAL_CPU_INFO holds /proc/cpuinfo data */
@@ -306,13 +304,13 @@ typedef struct PAL_CPU_INFO_ {
 
 typedef struct PAL_RANGE_INFO_ {
     PAL_NUM start;
-    PAL_NUM end; /* If end is not present, set this to UINT64_MAX as end marker. */
+    PAL_NUM end;
 } PAL_RANGE_INFO;
 
 typedef struct PAL_RES_RANGE_INFO_ {
     /* Count of total number of resources present. Eg. 0-63 will result in this count being 64 */
     PAL_NUM resource_count;
-    /* Count of total number of ranges present. Eg. 0-31,32-63, will result in this count being 2 */
+    /* Count of total number of ranges present. Eg. 0-31,32-63 will result in this count being 2 */
     PAL_NUM range_count;
     PAL_RANGE_INFO* ranges;
 } PAL_RES_RANGE_INFO;

Pal/include/host/Linux-common/topo_info.h

@@ -11,8 +11,8 @@
 /* Opens a pseudo-file describing HW resources such as online CPUs and counts the number of
  * HW resources present in the file (if count == true) and stores the result in `PAL_RES_RANGE_INFO`
  * struct if provided or simply reads the integer stored in the file (if count == false). If
- * `size_multiplier` is passed, then size qualifier like "K"/"M"/"G" are stored while reading the
- * integer. For example on a single-core machine, calling this function on
+ * `size_mult` is passed, then numerical representation of size qualifier like "K"/"M"/"G" is
+ * stored while reading the integer. For example on a single-core machine, calling this function on
  * `/sys/devices/system/cpu/online` with count == true will return 1 and 0 with count == false.
  * Returns UNIX error code on failure.
  * N.B: Understands complex formats like "1,3-5,6" when called with count == true.