[Mesa-dev] [PATCH] gallium/hud: bugfix: 68169 - Sensor extensions segfaults.

[Steven Toth]

https://bugs.freedesktop.org/show_bug.cgi?id=98169

Really a two part bug.

1. The recent extensions to the HUB framework were tested exclusively
with glxgears/demo/head. None of these tools exercise the
free_query_data() code path. When these codepaths were
tested, thanks to Christoph, using heaven-unigine
application, the sensors extension would segfault.

2. In a second case, same use case, extensions would
return zero values for cpufrequency, network stats etc.

On inspection, this was due to the 'unigine' application
created the HUD contexts twice (two screens?). The
extensions had never been tested in that configuration,
and the free_query_data() calls were thus trigger.

This patchset ensures that globally allocated
mechanisms for collecting and exposing various
disk, network, cpu frequency and sensors statistics
are retained correctly across multiple uses, and
free'd when no more callers require them.

Patchset was regression tested again with Unigine,
glxgears and glxdemo.

Tested-By: Christoph Haag <haagch at frickel.club>
Signed-off-by: Steven Toth <stoth at kernellabs.com>
---
 src/gallium/auxiliary/hud/hud_cpufreq.c      |  8 ++++++--
 src/gallium/auxiliary/hud/hud_diskstat.c     | 10 +++++++---
 src/gallium/auxiliary/hud/hud_nic.c          |  8 ++++++--
 src/gallium/auxiliary/hud/hud_sensors_temp.c | 18 +++++++++++++-----
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/src/gallium/auxiliary/hud/hud_cpufreq.c b/src/gallium/auxiliary/hud/hud_cpufreq.c
index 4501bbb..39bdcb0 100644
--- a/src/gallium/auxiliary/hud/hud_cpufreq.c
+++ b/src/gallium/auxiliary/hud/hud_cpufreq.c
@@ -57,6 +57,7 @@ struct cpufreq_info
    char sysfs_filename[128];
    uint64_t KHz;
    uint64_t last_time;
+   int users;
 };
 
 static int gcpufreq_count = 0;
@@ -116,8 +117,10 @@ static void
 free_query_data(void *p)
 {
    struct cpufreq_info *cfi = (struct cpufreq_info *)p;
-   list_del(&cfi->list);
-   FREE(cfi);
+   if (--cfi->users == 0) {
+      list_del(&cfi->list);
+      FREE(cfi);
+   }
 }
 
 /**
@@ -159,6 +162,7 @@ hud_cpufreq_graph_install(struct hud_pane *pane, int cpu_index,
       return;
    }
 
+   cfi->users++;
    gr->query_data = cfi;
    gr->query_new_value = query_cfi_load;
 
diff --git a/src/gallium/auxiliary/hud/hud_diskstat.c b/src/gallium/auxiliary/hud/hud_diskstat.c
index b248baf..000d6b3 100644
--- a/src/gallium/auxiliary/hud/hud_diskstat.c
+++ b/src/gallium/auxiliary/hud/hud_diskstat.c
@@ -73,6 +73,7 @@ struct diskstat_info
    char sysfs_filename[128];
    uint64_t last_time;
    struct stat_s last_stat;
+   int users;
 };
 
 /* TODO: We don't handle dynamic block device / partition
@@ -165,9 +166,11 @@ query_dsi_load(struct hud_graph *gr)
 static void
 free_query_data(void *p)
 {
-   struct diskstat_info *nic = (struct diskstat_info *) p;
-   list_del(&nic->list);
-   FREE(nic);
+   struct diskstat_info *dsi = (struct diskstat_info *) p;
+   if (--dsi->users == 0) {
+      list_del(&dsi->list);
+      FREE(dsi);
+   }
 }
 
 /**
@@ -205,6 +208,7 @@ hud_diskstat_graph_install(struct hud_pane *pane, const char *dev_name,
    else
       return;
 
+   dsi->users++;
    gr->query_data = dsi;
    gr->query_new_value = query_dsi_load;
 
diff --git a/src/gallium/auxiliary/hud/hud_nic.c b/src/gallium/auxiliary/hud/hud_nic.c
index fb6b8c0..e3f5910 100644
--- a/src/gallium/auxiliary/hud/hud_nic.c
+++ b/src/gallium/auxiliary/hud/hud_nic.c
@@ -59,6 +59,7 @@ struct nic_info
    char throughput_filename[128];
    uint64_t last_time;
    uint64_t last_nic_bytes;
+   int users;
 };
 
 /* TODO: We don't handle dynamic NIC arrival or removal.
@@ -238,8 +239,10 @@ static void
 free_query_data(void *p)
 {
    struct nic_info *nic = (struct nic_info *) p;
-   list_del(&nic->list);
-   FREE(nic);
+   if (--nic->users == 0) {
+      list_del(&nic->list);
+      FREE(nic);
+   }
 }
 
 /**
@@ -281,6 +284,7 @@ hud_nic_graph_install(struct hud_pane *pane, const char *nic_name,
    else
       return;
 
+   nic->users++;
    gr->query_data = nic;
    gr->query_new_value = query_nic_load;
 
diff --git a/src/gallium/auxiliary/hud/hud_sensors_temp.c b/src/gallium/auxiliary/hud/hud_sensors_temp.c
index e41b847..c6f7d5b 100644
--- a/src/gallium/auxiliary/hud/hud_sensors_temp.c
+++ b/src/gallium/auxiliary/hud/hud_sensors_temp.c
@@ -68,6 +68,7 @@ struct sensors_temp_info
    sensors_chip_name *chip;
    const sensors_feature *feature;
    double current, min, max, critical;
+   int users;
 };
 
 static double
@@ -193,11 +194,17 @@ static void
 free_query_data(void *p)
 {
    struct sensors_temp_info *sti = (struct sensors_temp_info *) p;
-   list_del(&sti->list);
-   if (sti->chip)
-      sensors_free_chip_name(sti->chip);
-   FREE(sti);
-   sensors_cleanup();
+   if (--sti->users == 0) {
+      list_del(&sti->list);
+      gsensors_temp_count--;
+      if (sti->chip)
+         sensors_free_chip_name(sti->chip);
+      FREE(sti);
+   }
+
+   /* Destroy sensor singleton when after all refs released. */
+   if (!gsensors_temp_count)
+      sensors_cleanup();
 }
 
 /**
@@ -234,6 +241,7 @@ hud_sensors_temp_graph_install(struct hud_pane *pane, const char *dev_name,
            sti->mode == SENSORS_POWER_CURRENT ? "Pow" :
            sti->mode == SENSORS_TEMP_CRITICAL ? "Crit" : "Unkn");
 
+   sti->users++;
    gr->query_data = sti;
    gr->query_new_value = query_sti_load;
 
-- 
2.7.4