device property: fix for a case of use-after-free

In device_remove_property_set(), the secondary fwnode needs
to be cleared before the pset is freed. This fixes a
use-after-free when a property set is providing the primary
fwnode.

As a result of the fix, the primary fwnode may end up
containing ERR_PTR(-ENODEV), so also adding checks for it to
the property handling code.

Reported-by: John Youn <John.Youn@synopsys.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

1 files changed, 17 insertions, 8 deletions

diff --git a/drivers/base/property.c b/drivers/base/property.c
index a163f2c59aa3..76628a7b45f1 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -218,7 +218,8 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
         bool ret;
 
         ret = __fwnode_property_present(fwnode, propname);
-        if (ret == false && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
+        if (ret == false && !IS_ERR_OR_NULL(fwnode) &&
+            !IS_ERR_OR_NULL(fwnode->secondary))
                 ret = __fwnode_property_present(fwnode->secondary, propname);
         return ret;
 }
@@ -423,7 +424,8 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
         int _ret_;                                                                      \
         _ret_ = FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_,              \
                                  _val_, _nval_);                                        \
-        if (_ret_ == -EINVAL && _fwnode_ && !IS_ERR_OR_NULL(_fwnode_->secondary))       \
+        if (_ret_ == -EINVAL && !IS_ERR_OR_NULL(_fwnode_) &&                            \
+            !IS_ERR_OR_NULL(_fwnode_->secondary))                                       \
                 _ret_ = FWNODE_PROP_READ(_fwnode_->secondary, _propname_, _type_,       \
                                 _proptype_, _val_, _nval_);                             \
         _ret_;                                                                          \
@@ -593,7 +595,8 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
         int ret;
 
         ret = __fwnode_property_read_string_array(fwnode, propname, val, nval);
-        if (ret == -EINVAL && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
+        if (ret == -EINVAL && !IS_ERR_OR_NULL(fwnode) &&
+            !IS_ERR_OR_NULL(fwnode->secondary))
                 ret = __fwnode_property_read_string_array(fwnode->secondary,
                                                           propname, val, nval);
         return ret;
@@ -621,7 +624,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
         int ret;
 
         ret = __fwnode_property_read_string(fwnode, propname, val);
-        if (ret == -EINVAL && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
+        if (ret == -EINVAL && !IS_ERR_OR_NULL(fwnode) &&
+            !IS_ERR_OR_NULL(fwnode->secondary))
                 ret = __fwnode_property_read_string(fwnode->secondary,
                                                     propname, val);
         return ret;
@@ -820,11 +824,16 @@ void device_remove_property_set(struct device *dev)
          * the pset. If there is no real firmware node (ACPI/DT) primary
          * will hold the pset.
          */
-        if (!is_pset_node(fwnode))
-                fwnode = fwnode->secondary;
-        if (!IS_ERR(fwnode) && is_pset_node(fwnode))
+        if (is_pset_node(fwnode)) {
+                set_primary_fwnode(dev, NULL);
                 pset_free_set(to_pset_node(fwnode));
-        set_secondary_fwnode(dev, NULL);
+        } else {
+                fwnode = fwnode->secondary;
+                if (!IS_ERR(fwnode) && is_pset_node(fwnode)) {
+                        set_secondary_fwnode(dev, NULL);
+                        pset_free_set(to_pset_node(fwnode));
+                }
+        }
 }
 EXPORT_SYMBOL_GPL(device_remove_property_set);