| Message ID | 20220614214024.3005275-1-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Clear the connection field properly | expand |
On Tue, Jun 14, 2022 at 10:40:24PM +0100, Suzuki Kuruppassery Poulose wrote: > coresight devices track their connections (output connections) and > hold a reference to the fwnode. When a device goes away, we walk through > the devices on the coresight bus and make sure that the references > are dropped. This happens both ways: > a) For all output connections from the device, drop the reference to > the target device via coresight_release_platform_data() > > b) Iterate over all the devices on the coresight bus and drop the > reference to fwnode if *this* device is the target of the output > connection, via coresight_remove_conns()->coresight_remove_match(). > > However, the coresight_remove_match() doesn't clear the fwnode field, > after dropping the reference, this causes use-after-free and > additional refcount drops on the fwnode. > > e.g., if we have two devices, A and B, with a connection, A -> B. > If we remove B first, B would clear the reference on B, from A > via coresight_remove_match(). But when A is removed, it still has > a connection with fwnode still pointing to B. Thus it tries to drops > the reference in coresight_release_platform_data(), raising the bells > like : > > [ 91.990153] ------------[ cut here ]------------ > [ 91.990163] refcount_t: addition on 0; use-after-free. > [ 91.990212] WARNING: CPU: 0 PID: 461 at lib/refcount.c:25 refcount_warn_saturate+0xa0/0x144 > [ 91.990260] Modules linked in: coresight_funnel coresight_replicator coresight_etm4x(-) > crct10dif_ce coresight ip_tables x_tables ipv6 [last unloaded: coresight_cpu_debug] > [ 91.990398] CPU: 0 PID: 461 Comm: rmmod Tainted: G W T 5.19.0-rc2+ #53 > [ 91.990418] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Feb 1 2019 > [ 91.990434] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 91.990454] pc : refcount_warn_saturate+0xa0/0x144 > [ 91.990476] lr : refcount_warn_saturate+0xa0/0x144 > [ 91.990496] sp : ffff80000c843640 > [ 91.990509] x29: ffff80000c843640 x28: ffff800009957c28 x27: ffff80000c8439a8 > [ 91.990560] x26: ffff00097eff1990 x25: ffff8000092b6ad8 x24: ffff00097eff19a8 > [ 91.990610] x23: ffff80000c8439a8 x22: 0000000000000000 x21: ffff80000c8439c2 > [ 91.990659] x20: 0000000000000000 x19: ffff00097eff1a10 x18: ffff80000ab99c40 > [ 91.990708] x17: 0000000000000000 x16: 0000000000000000 x15: ffff80000abf6fa0 > [ 91.990756] x14: 000000000000001d x13: 0a2e656572662d72 x12: 657466612d657375 > [ 91.990805] x11: 203b30206e6f206e x10: 6f69746964646120 x9 : ffff8000081aba28 > [ 91.990854] x8 : 206e6f206e6f6974 x7 : 69646461203a745f x6 : 746e756f63666572 > [ 91.990903] x5 : ffff00097648ec58 x4 : 0000000000000000 x3 : 0000000000000027 > [ 91.990952] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00080260ba00 > [ 91.991000] Call trace: > [ 91.991012] refcount_warn_saturate+0xa0/0x144 > [ 91.991034] kobject_get+0xac/0xb0 > [ 91.991055] of_node_get+0x2c/0x40 > [ 91.991076] of_fwnode_get+0x40/0x60 > [ 91.991094] fwnode_handle_get+0x3c/0x60 > [ 91.991116] fwnode_get_nth_parent+0xf4/0x110 > [ 91.991137] fwnode_full_name_string+0x48/0xc0 > [ 91.991158] device_node_string+0x41c/0x530 > [ 91.991178] pointer+0x320/0x3ec > [ 91.991198] vsnprintf+0x23c/0x750 > [ 91.991217] vprintk_store+0x104/0x4b0 > [ 91.991238] vprintk_emit+0x8c/0x360 > [ 91.991257] vprintk_default+0x44/0x50 > [ 91.991276] vprintk+0xcc/0xf0 > [ 91.991295] _printk+0x68/0x90 > [ 91.991315] of_node_release+0x13c/0x14c > [ 91.991334] kobject_put+0x98/0x114 > [ 91.991354] of_node_put+0x24/0x34 > [ 91.991372] of_fwnode_put+0x40/0x5c > [ 91.991390] fwnode_handle_put+0x38/0x50 > [ 91.991411] coresight_release_platform_data+0x74/0xb0 [coresight] > [ 91.991472] coresight_unregister+0x64/0xcc [coresight] > [ 91.991525] etm4_remove_dev+0x64/0x78 [coresight_etm4x] > [ 91.991563] etm4_remove_amba+0x1c/0x2c [coresight_etm4x] > [ 91.991598] amba_remove+0x3c/0x19c > > Reproducible by: (Build all coresight components as modules): > > #!/bin/sh > while true > do > for m in tmc stm cpu_debug etm4x replicator funnel > do > modprobe coresight_${m} > done > > for m in tmc stm cpu_debug etm4x replicator funnel > do > rmmode coresight_${m} > done > done > > Cc: stable@vger.kernel.org > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: Leo Yan <leo.yan@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> Looks good to me: Reviewed-by: Leo Yan <leo.yan@linaro.org> > --- > drivers/hwtracing/coresight/coresight-core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index ee6ce92ab4c3..1edfec1e9d18 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1424,6 +1424,7 @@ static int coresight_remove_match(struct device *dev, void *data) > * platform data. > */ > fwnode_handle_put(conn->child_fwnode); > + conn->child_fwnode = NULL; > /* No need to continue */ > break; > } > -- > 2.35.3 >
On Tue, Jun 14, 2022 at 10:40:24PM +0100, Suzuki K Poulose wrote: > coresight devices track their connections (output connections) and > hold a reference to the fwnode. When a device goes away, we walk through > the devices on the coresight bus and make sure that the references > are dropped. This happens both ways: > a) For all output connections from the device, drop the reference to > the target device via coresight_release_platform_data() > > b) Iterate over all the devices on the coresight bus and drop the > reference to fwnode if *this* device is the target of the output > connection, via coresight_remove_conns()->coresight_remove_match(). > > However, the coresight_remove_match() doesn't clear the fwnode field, > after dropping the reference, this causes use-after-free and > additional refcount drops on the fwnode. > > e.g., if we have two devices, A and B, with a connection, A -> B. > If we remove B first, B would clear the reference on B, from A > via coresight_remove_match(). But when A is removed, it still has > a connection with fwnode still pointing to B. Thus it tries to drops > the reference in coresight_release_platform_data(), raising the bells > like : > > [ 91.990153] ------------[ cut here ]------------ > [ 91.990163] refcount_t: addition on 0; use-after-free. > [ 91.990212] WARNING: CPU: 0 PID: 461 at lib/refcount.c:25 refcount_warn_saturate+0xa0/0x144 > [ 91.990260] Modules linked in: coresight_funnel coresight_replicator coresight_etm4x(-) > crct10dif_ce coresight ip_tables x_tables ipv6 [last unloaded: coresight_cpu_debug] > [ 91.990398] CPU: 0 PID: 461 Comm: rmmod Tainted: G W T 5.19.0-rc2+ #53 > [ 91.990418] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Feb 1 2019 > [ 91.990434] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 91.990454] pc : refcount_warn_saturate+0xa0/0x144 > [ 91.990476] lr : refcount_warn_saturate+0xa0/0x144 > [ 91.990496] sp : ffff80000c843640 > [ 91.990509] x29: ffff80000c843640 x28: ffff800009957c28 x27: ffff80000c8439a8 > [ 91.990560] x26: ffff00097eff1990 x25: ffff8000092b6ad8 x24: ffff00097eff19a8 > [ 91.990610] x23: ffff80000c8439a8 x22: 0000000000000000 x21: ffff80000c8439c2 > [ 91.990659] x20: 0000000000000000 x19: ffff00097eff1a10 x18: ffff80000ab99c40 > [ 91.990708] x17: 0000000000000000 x16: 0000000000000000 x15: ffff80000abf6fa0 > [ 91.990756] x14: 000000000000001d x13: 0a2e656572662d72 x12: 657466612d657375 > [ 91.990805] x11: 203b30206e6f206e x10: 6f69746964646120 x9 : ffff8000081aba28 > [ 91.990854] x8 : 206e6f206e6f6974 x7 : 69646461203a745f x6 : 746e756f63666572 > [ 91.990903] x5 : ffff00097648ec58 x4 : 0000000000000000 x3 : 0000000000000027 > [ 91.990952] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00080260ba00 > [ 91.991000] Call trace: > [ 91.991012] refcount_warn_saturate+0xa0/0x144 > [ 91.991034] kobject_get+0xac/0xb0 > [ 91.991055] of_node_get+0x2c/0x40 > [ 91.991076] of_fwnode_get+0x40/0x60 > [ 91.991094] fwnode_handle_get+0x3c/0x60 > [ 91.991116] fwnode_get_nth_parent+0xf4/0x110 > [ 91.991137] fwnode_full_name_string+0x48/0xc0 > [ 91.991158] device_node_string+0x41c/0x530 > [ 91.991178] pointer+0x320/0x3ec > [ 91.991198] vsnprintf+0x23c/0x750 > [ 91.991217] vprintk_store+0x104/0x4b0 > [ 91.991238] vprintk_emit+0x8c/0x360 > [ 91.991257] vprintk_default+0x44/0x50 > [ 91.991276] vprintk+0xcc/0xf0 > [ 91.991295] _printk+0x68/0x90 > [ 91.991315] of_node_release+0x13c/0x14c > [ 91.991334] kobject_put+0x98/0x114 > [ 91.991354] of_node_put+0x24/0x34 > [ 91.991372] of_fwnode_put+0x40/0x5c > [ 91.991390] fwnode_handle_put+0x38/0x50 > [ 91.991411] coresight_release_platform_data+0x74/0xb0 [coresight] > [ 91.991472] coresight_unregister+0x64/0xcc [coresight] > [ 91.991525] etm4_remove_dev+0x64/0x78 [coresight_etm4x] > [ 91.991563] etm4_remove_amba+0x1c/0x2c [coresight_etm4x] > [ 91.991598] amba_remove+0x3c/0x19c > > Reproducible by: (Build all coresight components as modules): > > #!/bin/sh > while true > do > for m in tmc stm cpu_debug etm4x replicator funnel > do > modprobe coresight_${m} > done > > for m in tmc stm cpu_debug etm4x replicator funnel > do > rmmode coresight_${m} > done > done > > Cc: stable@vger.kernel.org > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: Mike Leach <mike.leach@linaro.org> > Cc: Leo Yan <leo.yan@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/hwtracing/coresight/coresight-core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c > index ee6ce92ab4c3..1edfec1e9d18 100644 > --- a/drivers/hwtracing/coresight/coresight-core.c > +++ b/drivers/hwtracing/coresight/coresight-core.c > @@ -1424,6 +1424,7 @@ static int coresight_remove_match(struct device *dev, void *data) > * platform data. > */ > fwnode_handle_put(conn->child_fwnode); > + conn->child_fwnode = NULL; > /* No need to continue */ I have applied this patch. Thanks, Mathieu > break; > } > -- > 2.35.3 >
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ee6ce92ab4c3..1edfec1e9d18 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1424,6 +1424,7 @@ static int coresight_remove_match(struct device *dev, void *data) * platform data. */ fwnode_handle_put(conn->child_fwnode); + conn->child_fwnode = NULL; /* No need to continue */ break; }
coresight devices track their connections (output connections) and hold a reference to the fwnode. When a device goes away, we walk through the devices on the coresight bus and make sure that the references are dropped. This happens both ways: a) For all output connections from the device, drop the reference to the target device via coresight_release_platform_data() b) Iterate over all the devices on the coresight bus and drop the reference to fwnode if *this* device is the target of the output connection, via coresight_remove_conns()->coresight_remove_match(). However, the coresight_remove_match() doesn't clear the fwnode field, after dropping the reference, this causes use-after-free and additional refcount drops on the fwnode. e.g., if we have two devices, A and B, with a connection, A -> B. If we remove B first, B would clear the reference on B, from A via coresight_remove_match(). But when A is removed, it still has a connection with fwnode still pointing to B. Thus it tries to drops the reference in coresight_release_platform_data(), raising the bells like : [ 91.990153] ------------[ cut here ]------------ [ 91.990163] refcount_t: addition on 0; use-after-free. [ 91.990212] WARNING: CPU: 0 PID: 461 at lib/refcount.c:25 refcount_warn_saturate+0xa0/0x144 [ 91.990260] Modules linked in: coresight_funnel coresight_replicator coresight_etm4x(-) crct10dif_ce coresight ip_tables x_tables ipv6 [last unloaded: coresight_cpu_debug] [ 91.990398] CPU: 0 PID: 461 Comm: rmmod Tainted: G W T 5.19.0-rc2+ #53 [ 91.990418] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Feb 1 2019 [ 91.990434] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 91.990454] pc : refcount_warn_saturate+0xa0/0x144 [ 91.990476] lr : refcount_warn_saturate+0xa0/0x144 [ 91.990496] sp : ffff80000c843640 [ 91.990509] x29: ffff80000c843640 x28: ffff800009957c28 x27: ffff80000c8439a8 [ 91.990560] x26: ffff00097eff1990 x25: ffff8000092b6ad8 x24: ffff00097eff19a8 [ 91.990610] x23: ffff80000c8439a8 x22: 0000000000000000 x21: ffff80000c8439c2 [ 91.990659] x20: 0000000000000000 x19: ffff00097eff1a10 x18: ffff80000ab99c40 [ 91.990708] x17: 0000000000000000 x16: 0000000000000000 x15: ffff80000abf6fa0 [ 91.990756] x14: 000000000000001d x13: 0a2e656572662d72 x12: 657466612d657375 [ 91.990805] x11: 203b30206e6f206e x10: 6f69746964646120 x9 : ffff8000081aba28 [ 91.990854] x8 : 206e6f206e6f6974 x7 : 69646461203a745f x6 : 746e756f63666572 [ 91.990903] x5 : ffff00097648ec58 x4 : 0000000000000000 x3 : 0000000000000027 [ 91.990952] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00080260ba00 [ 91.991000] Call trace: [ 91.991012] refcount_warn_saturate+0xa0/0x144 [ 91.991034] kobject_get+0xac/0xb0 [ 91.991055] of_node_get+0x2c/0x40 [ 91.991076] of_fwnode_get+0x40/0x60 [ 91.991094] fwnode_handle_get+0x3c/0x60 [ 91.991116] fwnode_get_nth_parent+0xf4/0x110 [ 91.991137] fwnode_full_name_string+0x48/0xc0 [ 91.991158] device_node_string+0x41c/0x530 [ 91.991178] pointer+0x320/0x3ec [ 91.991198] vsnprintf+0x23c/0x750 [ 91.991217] vprintk_store+0x104/0x4b0 [ 91.991238] vprintk_emit+0x8c/0x360 [ 91.991257] vprintk_default+0x44/0x50 [ 91.991276] vprintk+0xcc/0xf0 [ 91.991295] _printk+0x68/0x90 [ 91.991315] of_node_release+0x13c/0x14c [ 91.991334] kobject_put+0x98/0x114 [ 91.991354] of_node_put+0x24/0x34 [ 91.991372] of_fwnode_put+0x40/0x5c [ 91.991390] fwnode_handle_put+0x38/0x50 [ 91.991411] coresight_release_platform_data+0x74/0xb0 [coresight] [ 91.991472] coresight_unregister+0x64/0xcc [coresight] [ 91.991525] etm4_remove_dev+0x64/0x78 [coresight_etm4x] [ 91.991563] etm4_remove_amba+0x1c/0x2c [coresight_etm4x] [ 91.991598] amba_remove+0x3c/0x19c Reproducible by: (Build all coresight components as modules): #!/bin/sh while true do for m in tmc stm cpu_debug etm4x replicator funnel do modprobe coresight_${m} done for m in tmc stm cpu_debug etm4x replicator funnel do rmmode coresight_${m} done done Cc: stable@vger.kernel.org Cc: Mathieu Poirier <mathieu.poirier@linaro.org> Cc: Mike Leach <mike.leach@linaro.org> Cc: Leo Yan <leo.yan@linaro.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- drivers/hwtracing/coresight/coresight-core.c | 1 + 1 file changed, 1 insertion(+)