library: the uref functions were insufficiently robust

The earlier attempt at protecting these functions from
already freed memory worked just fine until the memory
was, in fact, reused by the OS. At that point, the ref
count would most likely fail an existing a test for 0.

So this commit will take control of the 'info' pointer
and force it to NULL when a reference count reaches 0.

Plus, since it makes little sense returning an address
that a caller already has, henceforth we will return a
reference count out of the 'ref' and 'unref functions.

Reference(s):
commit 74beff80ff3624761ecd20fafac0485f48c9c873

Signed-off-by: Jim Warner <james.warner@comcast.net>

diff --git a/proc/meminfo.c b/proc/meminfo.c
index 6cfe808c3c0a540f9ef9c68a68e30028e3cddd49..f373b7720c09969393ab8947c64a04cc20caaa7b 100644 (file)
--- a/proc/meminfo.c
+++ b/proc/meminfo.c
@@ -214,25 +214,27 @@ PROCPS_EXPORT int procps_meminfo_read (
 }
 
 
-PROCPS_EXPORT struct procps_meminfo *procps_meminfo_ref (
+PROCPS_EXPORT int procps_meminfo_ref (
         struct procps_meminfo *info)
 {
     if (info == NULL)
-        return NULL;
+        return -EINVAL;
     info->refcount++;
-    return info;
+    return info->refcount;
 }
 
-PROCPS_EXPORT struct procps_meminfo *procps_meminfo_unref (
-        struct procps_meminfo *info)
+PROCPS_EXPORT int procps_meminfo_unref (
+        struct procps_meminfo **info)
 {
-    if (info == NULL || info->refcount == 0)
-        return NULL;
-    info->refcount--;
-    if (info->refcount > 0)
-        return info;
-    free(info);
-    return NULL;
+    if (info == NULL || *info == NULL)
+        return -EINVAL;
+    (*info)->refcount--;
+    if ((*info)->refcount == 0) {
+        free(*info);
+        *info = NULL;
+        return 0;
+    }
+    return (*info)->refcount;
 }
 
 /* Accessor functions */

diff --git a/proc/meminfo.h b/proc/meminfo.h
index 31b46d45eb48091c225b8681069003e2eff0819e..d394893a63249a42a834ed0134e8403cb189d5f9 100644 (file)
--- a/proc/meminfo.h
+++ b/proc/meminfo.h
@@ -58,8 +58,8 @@ struct procps_meminfo;
 int procps_meminfo_new (struct procps_meminfo **info);
 int procps_meminfo_read (struct procps_meminfo *info);
 
-struct procps_meminfo *procps_meminfo_ref (struct procps_meminfo *info);
-struct procps_meminfo *procps_meminfo_unref (struct procps_meminfo *info);
+int procps_meminfo_ref (struct procps_meminfo *info);
+int procps_meminfo_unref (struct procps_meminfo **info);
 
 unsigned long procps_meminfo_get (struct procps_meminfo *info, enum meminfo_item item);
 int procps_meminfo_get_chain (struct procps_meminfo *info, struct meminfo_result *item);

diff --git a/proc/readstat.c b/proc/readstat.c
index 48c323f4458ac7af37ae28cd736eeee7f22ce213..6af84f410464130a14d2e35a2976a953aea75761 100644 (file)
--- a/proc/readstat.c
+++ b/proc/readstat.c
@@ -152,27 +152,29 @@ PROCPS_EXPORT int procps_stat_read (
     return 0;
 }
 
-PROCPS_EXPORT struct procps_statinfo *procps_stat_ref (
+PROCPS_EXPORT int procps_stat_ref (
         struct procps_statinfo *info)
 {
     if (info == NULL)
-        return NULL;
+        return -EINVAL;
     info->refcount++;
-    return info;
+    return info->refcount;
 }
 
-PROCPS_EXPORT struct procps_statinfo *procps_stat_unref (
-        struct procps_statinfo *info)
+PROCPS_EXPORT int procps_stat_unref (
+        struct procps_statinfo **info)
 {
-    if (info == NULL || info->refcount == 0)
-        return NULL;
-    info->refcount--;
-    if (info->refcount > 0)
-        return info;
-    if (info->jiff_hists)
-        free(info->jiff_hists);
-    free(info);
-    return NULL;
+    if (info == NULL || *info == NULL)
+        return -EINVAL;
+    (*info)->refcount--;
+    if ((*info)->refcount == 0) {
+        if ((*info)->jiff_hists != NULL)
+            free((*info)->jiff_hists);
+        free(*info);
+        *info = NULL;
+        return 0;
+    }
+    return (*info)->refcount;
 }
 
 PROCPS_EXPORT jiff procps_stat_get_cpu (

diff --git a/proc/readstat.h b/proc/readstat.h
index d155524a8161d68cddb30d280bd30294805e3d18..902f5ee36f0308aac62f4e749158036e6c4cf838 100644 (file)
--- a/proc/readstat.h
+++ b/proc/readstat.h
@@ -74,8 +74,8 @@ int procps_stat_new (struct procps_statinfo **info);
 int procps_stat_read (struct procps_statinfo *info, const int cpu_only);
 int procps_stat_read_jiffs (struct procps_statinfo *info);
 
-struct procps_statinfo *procps_stat_ref (struct procps_statinfo *info);
-struct procps_statinfo *procps_stat_unref (struct procps_statinfo *info);
+int procps_stat_ref (struct procps_statinfo *info);
+int procps_stat_unref (struct procps_statinfo **info);
 
 jiff procps_stat_get_cpu (struct procps_statinfo *info, enum procps_cpu_item item);
 int procps_stat_get_cpu_chain (struct procps_statinfo *info, struct procps_cpu_result *item);

diff --git a/proc/vmstat.c b/proc/vmstat.c
index 953c8b6a074be43b63b27dcb912d5ad9863ef8cc..775cf1fd5834dc4c2cc55520cd471e9538a4de44 100644 (file)
--- a/proc/vmstat.c
+++ b/proc/vmstat.c
@@ -125,25 +125,27 @@ PROCPS_EXPORT int procps_vmstat_read (
     return 0;
 }
 
-PROCPS_EXPORT struct procps_vmstat *procps_vmstat_ref (
+PROCPS_EXPORT int procps_vmstat_ref (
         struct procps_vmstat *info)
 {
     if (info == NULL)
-        return NULL;
+        return -EINVAL;
     info->refcount++;
-    return info;
+    return info->refcount;
 }
 
-PROCPS_EXPORT struct procps_vmstat *procps_vmstat_unref (
-        struct procps_vmstat *info)
+PROCPS_EXPORT int procps_vmstat_unref (
+        struct procps_vmstat **info)
 {
-    if (info == NULL || info->refcount == 0)
-        return NULL;
-    info->refcount--;
-    if (info->refcount > 0)
-        return info;
-    free(info);
-    return NULL;
+    if (info == NULL || *info == NULL)
+        return -EINVAL;
+    (*info)->refcount--;
+    if ((*info)->refcount == 0) {
+        free(*info);
+        *info = NULL;
+        return 0;
+    }
+    return (*info)->refcount;
 }
 
 /* Accessor functions */

diff --git a/proc/vmstat.h b/proc/vmstat.h
index 7c22bd7154a429ccfa8563fbb126c8b18d390c92..4b97df451e2e068137ea37d934cc866f060cd2ad 100644 (file)
--- a/proc/vmstat.h
+++ b/proc/vmstat.h
@@ -45,8 +45,8 @@ struct procps_vmstat;
 int procps_vmstat_new (struct procps_vmstat **info);
 int procps_vmstat_read (struct procps_vmstat *info);
 
-struct procps_vmstat *procps_vmstat_ref (struct procps_vmstat *info);
-struct procps_vmstat *procps_vmstat_unref (struct procps_vmstat *info);
+int procps_vmstat_ref (struct procps_vmstat *info);
+int procps_vmstat_unref (struct procps_vmstat **info);
 
 unsigned long procps_vmstat_get (struct procps_vmstat *info, enum vmstat_item item);
 int procps_vmstat_get_chain (struct procps_vmstat *info, struct vmstat_result *item);

diff --git a/ps/output.c b/ps/output.c
index 05b0c11a4257924a7843723a61eb43a64d88f96f..72be9235931f12f5ac1bac4821914f204f34ba7d 100644 (file)
--- a/ps/output.c
+++ b/ps/output.c
@@ -106,7 +106,7 @@ static void get_boot_time(void)
        xerrx(EXIT_FAILURE,
                _("Unable to read system stat information"));
     boot_time = procps_stat_get_sys(sys_info, PROCPS_STAT_BTIME);
-    procps_stat_unref(sys_info);
+    procps_stat_unref(&sys_info);
 }
 
 static void get_memory_total()
@@ -119,7 +119,7 @@ static void get_memory_total()
        xerrx(EXIT_FAILURE,
                _("Unable to read meminfo information"));
     memory_total = procps_meminfo_get(mem_info, PROCPS_MEM_TOTAL);
-    procps_meminfo_unref(mem_info);
+    procps_meminfo_unref(&mem_info);
 }
 
 /*************************************************************************/

diff --git a/top/top.c b/top/top.c
index 9b66c6a33be0487aec8679740d8d3344d07896ec..d0a691dc6dd5508aebbe7fa33f922e0837117a72 100644 (file)
--- a/top/top.c
+++ b/top/top.c
@@ -578,8 +578,8 @@ static void bye_bye (const char *str) {
 #endif // end: ATEOJ_RPTHSH
 #endif // end: OFF_HST_HASH
 
-   procps_stat_unref(sys_info);
-   procps_meminfo_unref(mem_info);
+   procps_stat_unref(&sys_info);
+   procps_meminfo_unref(&mem_info);
 #ifndef NUMA_DISABLE
   if (Libnuma_handle) dlclose(Libnuma_handle);
 #endif