Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1339600 > unrolled thread

[PATCH 0/2] device property: fix for couple of bugs

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] device property: fix for couple of bugs Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-02-22 16:00 +0100
    [PATCH 1/2] device property: fwnode->secondary may contain ERR_PTR(-ENODEV) Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-02-22 16:00 +0100
    Re: [PATCH 0/2] device property: fix for couple of bugs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-24 00:40 +0100
      Re: [PATCH 0/2] device property: fix for couple of bugs Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-02-26 09:10 +0100

#1339600 — [PATCH 0/2] device property: fix for couple of bugs

Hi,

The fwnode->secondary is causing problems when a property_set is
providing the primary fwnode.

Both of these fixes are a bit clumsy looking IMO, but I didn't have
better ideas. I think the second one fixing the use-after-free bug
should ideally be taken care of in set_secondary_fwnode() instead of
device_remove_property_set(), but I didn't have any ideas how to do
that.


Heikki Krogerus (2):
  device property: fwnode->secondary may contain ERR_PTR(-ENODEV)
  device property: fix for a case of use-after-free

 drivers/base/property.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.7.0

[toc] | [next] | [standalone]

#1339607 — [PATCH 1/2] device property: fwnode->secondary may contain ERR_PTR(-ENODEV)

This fixes BUG that is trickered when fwnode->secondary is not null,
but has ERR_PTR(-ENODEV) instead.

BUG: unable to handle kernel paging request at ffffffffffffffed
IP: [<ffffffff81677b86>] __fwnode_property_read_string+0x26/0x160
PGD 200e067 PUD 2010067 PMD 0
Oops: 0000 [#1] SMP KASAN
Modules linked in: dwc3_pci(+) dwc3
CPU: 0 PID: 1138 Comm: modprobe Not tainted 4.5.0-rc5+ #61
task: ffff88015aaf5b00 ti: ffff88007b958000 task.ti: ffff88007b958000
RIP: 0010:[<ffffffff81677b86>]  [<ffffffff81677b86>] __fwnode_property_read_string+0x26/0x160
RSP: 0018:ffff88007b95eff8  EFLAGS: 00010246
RAX: fffffbfffffffffd RBX: ffffffffffffffed RCX: ffff88015999cd37
RDX: dffffc0000000000 RSI: ffffffff81e11bc0 RDI: ffffffffffffffed
RBP: ffff88007b95f020 R08: 0000000000000000 R09: 0000000000000000
R10: ffff88007b90f7cf R11: 0000000000000000 R12: ffff88007b95f0a0
R13: 00000000fffffffa R14: ffffffff81e11bc0 R15: ffff880159ea37a0
FS:  00007ff35f46c700(0000) GS:ffff88015b800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: ffffffffffffffed CR3: 000000007b8be000 CR4: 00000000001006f0
Stack:
 ffff88015999cd20 ffffffff81e11bc0 ffff88007b95f0a0 ffff88007b383dd8
 ffff880159ea37a0 ffff88007b95f048 ffffffff81677d03 ffff88007b952460
 ffffffff81e11bc0 ffff88007b95f0a0 ffff88007b95f070 ffffffff81677d40
Call Trace:
 [<ffffffff81677d03>] fwnode_property_read_string+0x43/0x50
 [<ffffffff81677d40>] device_property_read_string+0x30/0x40
...

Fixes: 362c0b30249e (device property: Fallback to secondary fwnode if primary misses the property)
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/base/property.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c359351..a163f2c 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -218,7 +218,7 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
 	bool ret;
 
 	ret = __fwnode_property_present(fwnode, propname);
-	if (ret == false && fwnode && fwnode->secondary)
+	if (ret == false && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
 		ret = __fwnode_property_present(fwnode->secondary, propname);
 	return ret;
 }
@@ -423,7 +423,7 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 	int _ret_;									\
 	_ret_ = FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_,		\
 				 _val_, _nval_);					\
-	if (_ret_ == -EINVAL && _fwnode_ && _fwnode_->secondary)			\
+	if (_ret_ == -EINVAL && _fwnode_ && !IS_ERR_OR_NULL(_fwnode_->secondary))	\
 		_ret_ = FWNODE_PROP_READ(_fwnode_->secondary, _propname_, _type_,	\
 				_proptype_, _val_, _nval_);				\
 	_ret_;										\
@@ -593,7 +593,7 @@ 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 && fwnode->secondary)
+	if (ret == -EINVAL && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
 		ret = __fwnode_property_read_string_array(fwnode->secondary,
 							  propname, val, nval);
 	return ret;
@@ -621,7 +621,7 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 	int ret;
 
 	ret = __fwnode_property_read_string(fwnode, propname, val);
-	if (ret == -EINVAL && fwnode && fwnode->secondary)
+	if (ret == -EINVAL && fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
 		ret = __fwnode_property_read_string(fwnode->secondary,
 						    propname, val);
 	return ret;
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1341173

On Monday, February 22, 2016 04:50:39 PM Heikki Krogerus wrote:
> Hi,
> 
> The fwnode->secondary is causing problems when a property_set is
> providing the primary fwnode.
> 
> Both of these fixes are a bit clumsy looking IMO, but I didn't have
> better ideas. I think the second one fixing the use-after-free bug
> should ideally be taken care of in set_secondary_fwnode() instead of
> device_remove_property_set(), but I didn't have any ideas how to do
> that.

Can you please CC device property framework patches to linux-acpi?

They are somewhat easier to handle this way.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1343891

On Wed, Feb 24, 2016 at 12:37:43AM +0100, Rafael J. Wysocki wrote:
> On Monday, February 22, 2016 04:50:39 PM Heikki Krogerus wrote:
> > Hi,
> > 
> > The fwnode->secondary is causing problems when a property_set is
> > providing the primary fwnode.
> > 
> > Both of these fixes are a bit clumsy looking IMO, but I didn't have
> > better ideas. I think the second one fixing the use-after-free bug
> > should ideally be taken care of in set_secondary_fwnode() instead of
> > device_remove_property_set(), but I didn't have any ideas how to do
> > that.
> 
> Can you please CC device property framework patches to linux-acpi?
> 
> They are somewhat easier to handle this way.

Will do.

Thanks,

-- 
heikki

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web