| Message ID | 1457607798-62438-1-git-send-email-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On Thursday, March 10, 2016 01:03:18 PM Heikki Krogerus wrote: > 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> OK, queued up for 4.6, thanks! > --- > drivers/base/property.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index d6c8caa..9b1a65d 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; > @@ -816,11 +820,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); > >
diff --git a/drivers/base/property.c b/drivers/base/property.c index d6c8caa..9b1a65d 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; @@ -816,11 +820,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);
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> --- drivers/base/property.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)