drivers: thermal: int340x_thermal: Fix sysfs race condition
authorAaron Hill <aa1ronham@gmail.com>
Mon, 24 Dec 2018 19:23:36 +0000 (14:23 -0500)
committerZhang Rui <rui.zhang@intel.com>
Fri, 18 Jan 2019 07:23:04 +0000 (15:23 +0800)
Changes since V1:
* Use dev_info instead of printk
* Use dev_warn instead of BUG_ON

Previously, sysfs_create_group was called before all initialization had
fully run - specifically, before pci_set_drvdata was called. Since the
sysctl group is visible to userspace as soon as sysfs_create_group
returns, a small window of time existed during which a process could read
from an uninitialized/partially-initialized device.

This commit moves the creation of the sysctl group to after all
initialized is completed. This ensures that it's impossible for
userspace to read from a sysctl file before initialization has fully
completed.

To catch any future regressions, I've added a check to ensure
that proc_thermal_emum_mode is never PROC_THERMAL_NONE when a process
tries to read from a sysctl file. Previously, the aforementioned race
condition could result in the 'else' branch
running while PROC_THERMAL_NONE was set,
leading to a null pointer deference.

Signed-off-by: Aaron Hill <aa1ronham@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
drivers/thermal/intel/int340x_thermal/processor_thermal_device.c

index 8e0f665cf06f89bbe995bd46779764f3e0d00ddf..8e1cf4d789be10df2413e1311bba63dde3545f43 100644 (file)
@@ -84,7 +84,12 @@ static ssize_t power_limit_##index##_##suffix##_show(struct device *dev, \
        struct pci_dev *pci_dev; \
        struct platform_device *pdev; \
        struct proc_thermal_device *proc_dev; \
-\
+       \
+       if (proc_thermal_emum_mode == PROC_THERMAL_NONE) { \
+               dev_warn(dev, "Attempted to get power limit before device was initialized!\n"); \
+               return 0; \
+       } \
+       \
        if (proc_thermal_emum_mode == PROC_THERMAL_PLATFORM_DEV) { \
                pdev = to_platform_device(dev); \
                proc_dev = platform_get_drvdata(pdev); \
@@ -298,11 +303,6 @@ static int proc_thermal_add(struct device *dev,
        *priv = proc_priv;
 
        ret = proc_thermal_read_ppcc(proc_priv);
-       if (!ret) {
-               ret = sysfs_create_group(&dev->kobj,
-                                        &power_limit_attribute_group);
-
-       }
        if (ret)
                return ret;
 
@@ -316,8 +316,7 @@ static int proc_thermal_add(struct device *dev,
 
        proc_priv->int340x_zone = int340x_thermal_zone_add(adev, ops);
        if (IS_ERR(proc_priv->int340x_zone)) {
-               ret = PTR_ERR(proc_priv->int340x_zone);
-               goto remove_group;
+               return PTR_ERR(proc_priv->int340x_zone);
        } else
                ret = 0;
 
@@ -331,9 +330,6 @@ static int proc_thermal_add(struct device *dev,
 
 remove_zone:
        int340x_thermal_zone_remove(proc_priv->int340x_zone);
-remove_group:
-       sysfs_remove_group(&proc_priv->dev->kobj,
-                          &power_limit_attribute_group);
 
        return ret;
 }
@@ -364,7 +360,10 @@ static int int3401_add(struct platform_device *pdev)
        platform_set_drvdata(pdev, proc_priv);
        proc_thermal_emum_mode = PROC_THERMAL_PLATFORM_DEV;
 
-       return 0;
+       dev_info(&pdev->dev, "Creating sysfs group for PROC_THERMAL_PLATFORM_DEV\n");
+
+       return sysfs_create_group(&pdev->dev.kobj,
+                                        &power_limit_attribute_group);
 }
 
 static int int3401_remove(struct platform_device *pdev)
@@ -441,7 +440,10 @@ static int  proc_thermal_pci_probe(struct pci_dev *pdev,
                        dev_err(&pdev->dev, "No auxiliary DTSs enabled\n");
        }
 
-       return 0;
+       dev_info(&pdev->dev, "Creating sysfs group for PROC_THERMAL_PCI\n");
+
+       return sysfs_create_group(&pdev->dev.kobj,
+                                        &power_limit_attribute_group);
 }
 
 static void  proc_thermal_pci_remove(struct pci_dev *pdev)