From 9a4e7849b5e4e8742d71fa90fbf0722dd0710a56 Mon Sep 17 00:00:00 2001 From: Russell King Date: Tue, 26 Jan 2016 14:49:22 +0000 Subject: component: fix crash on x86_64 with hda audio drivers Maarten reports that the addition of releasing match data to the component helper results in a general protection fault on x86_64. This is caused by the devm resources being freed in reverse order to their allocation, which caused a use-after-free of the match array. Switch the match array to be a more conventional kmalloc/kfree() affair, explicitly freeing it along with the parent match data structure. Reported-by: Maarten Lankhorst Fixes: ce657b1cddf1 ("component: add support for releasing match data") Signed-off-by: Russell King diff --git a/drivers/base/component.c b/drivers/base/component.c index 89f5cf68..05cd26c 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -206,6 +206,8 @@ static void component_match_release(struct device *master, if (mc->release) mc->release(master, mc->data); } + + kfree(match->compare); } static void devm_component_match_release(struct device *dev, void *res) @@ -221,14 +223,14 @@ static int component_match_realloc(struct device *dev, if (match->alloc == num) return 0; - new = devm_kmalloc_array(dev, num, sizeof(*new), GFP_KERNEL); + new = kmalloc_array(num, sizeof(*new), GFP_KERNEL); if (!new) return -ENOMEM; if (match->compare) { memcpy(new, match->compare, sizeof(*new) * min(match->num, num)); - devm_kfree(dev, match->compare); + kfree(match->compare); } match->compare = new; match->alloc = num; -- cgit v0.10.2 From 57480484f9f7631738ef28b952eca3c9081c4291 Mon Sep 17 00:00:00 2001 From: "Jon Medhurst (Tixy)" Date: Tue, 26 Jan 2016 17:59:13 +0000 Subject: component: Detach components when deleting master struct component_master_add_with_match calls find_components which, if any components already exist, it attaches to the master struct. However, if we later encounter an error the master struct is deleted, leaving components with a dangling pointer to it. If the error was a temporary one, e.g. for probe deferral, then when the master device is re-probed, it will fail to find the required components as they appear to already be attached to a master. Fix this by nulling components pointers to the master struct when it is deleted. This code is factored out into a separate function so it can be shared with component_master_del. Signed-off-by: Jon Medhurst Signed-off-by: Russell King diff --git a/drivers/base/component.c b/drivers/base/component.c index 05cd26c..2738039 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -285,6 +285,24 @@ void component_match_add_release(struct device *master, } EXPORT_SYMBOL(component_match_add_release); +static void free_master(struct master *master) +{ + struct component_match *match = master->match; + int i; + + list_del(&master->node); + + if (match) { + for (i = 0; i < match->num; i++) { + struct component *c = match->compare[i].component; + if (c) + c->master = NULL; + } + } + + kfree(master); +} + int component_master_add_with_match(struct device *dev, const struct component_master_ops *ops, struct component_match *match) @@ -311,11 +329,9 @@ int component_master_add_with_match(struct device *dev, ret = try_to_bring_up_master(master, NULL); - if (ret < 0) { - /* Delete off the list if we weren't successful */ - list_del(&master->node); - kfree(master); - } + if (ret < 0) + free_master(master); + mutex_unlock(&component_mutex); return ret < 0 ? ret : 0; @@ -326,25 +342,12 @@ void component_master_del(struct device *dev, const struct component_master_ops *ops) { struct master *master; - int i; mutex_lock(&component_mutex); master = __master_find(dev, ops); if (master) { - struct component_match *match = master->match; - take_down_master(master); - - list_del(&master->node); - - if (match) { - for (i = 0; i < match->num; i++) { - struct component *c = match->compare[i].component; - if (c) - c->master = NULL; - } - } - kfree(master); + free_master(master); } mutex_unlock(&component_mutex); } -- cgit v0.10.2 From 8e7199c2c50fff1969302643171eaa33f1ca148f Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Mon, 8 Feb 2016 21:12:58 +0000 Subject: component: remove device from master match list on failed add Calling component_add() may result in the completion of a set of devices, which will try to bring up a master. In bringing the master up, we populate its match array with the current set of children. If binding any of the devices fails, component_add() itself will fail, free the struct component entry, and return to the caller. The now-freed entry is never removed from the master's match array, and will later be used in a futile attempt to bind to freed memory. Bring component_add's behaviour on failure to bring up a master into line with component_del by removing the (to-be-freed) component from the master's match array. The specific case which broke was: - rockchip_drm_drv adds a component master - dwhdmi_rockchip adds a child component in probe (master incomplete) - rockchip_drm_vop adds two children in probe, which completes the set - inside component_add, we try to bring up the master, having populated the master's match array, and fail with EPROBE_DEFER from dwhdmi_rockchip; we delete the putative component - rockchip_drm_vop's probe fails and returns EPROBE_DEFER - we later re-probe rockchip_drm_vop and add the component; the master is complete, so we attempt to bring it up again - walking the match array, we find the previous child, whose master pointer doesn't match (as it has been freed in the meantime) - rockchip_drm_vop probe fails, and will never be attempted again Fixes: ffc30b74fd6d01588bd3fdebc3b1acc0857e6fc8 Signed-off-by: Daniel Stone Cc: Russell King Cc: Thierry Reding Cc: Laurent Pinchart Signed-off-by: Russell King diff --git a/drivers/base/component.c b/drivers/base/component.c index 2738039..04a1582 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -491,6 +491,8 @@ int component_add(struct device *dev, const struct component_ops *ops) ret = try_to_bring_up_masters(component); if (ret < 0) { + if (component->master) + remove_component(component->master, component); list_del(&component->node); kfree(component); -- cgit v0.10.2