* [PATCH 1/4] fpga: stratix10-soc: fix use-after-free on s10_init()
2019-05-09 21:08 [PATCH 0/4] patches for FPGA Alan Tull
@ 2019-05-09 21:08 ` Alan Tull
2019-05-09 21:08 ` [PATCH 2/4] fpga: dfl: afu: Pass the correct device to dma_mapping_error() Alan Tull
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Alan Tull @ 2019-05-09 21:08 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Moritz Fischer, Alan Tull, linux-kernel, linux-fpga, Wen Yang,
Nicolas Saenz Julienne
From: Wen Yang <wen.yang99@zte.com.cn>
The refcount of fw_np has already been decreased by of_find_matching_node()
so it shouldn't be used anymore.
This patch adds an of_node_get() before of_find_matching_node() to avoid
the use-after-free problem.
Fixes: e7eef1d7633a ("fpga: add intel stratix10 soc fpga manager driver")
Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
Cc: Alan Tull <atull@kernel.org>
Cc: Moritz Fischer <mdf@kernel.org>
Cc: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Cc: linux-fpga@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Moritz Fischer <mdf@kernel.org>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Alan Tull <atull@kernel.org>
---
drivers/fpga/stratix10-soc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 13851b3d1c56..215d33789c74 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -507,12 +507,16 @@ static int __init s10_init(void)
if (!fw_np)
return -ENODEV;
+ of_node_get(fw_np);
np = of_find_matching_node(fw_np, s10_of_match);
- if (!np)
+ if (!np) {
+ of_node_put(fw_np);
return -ENODEV;
+ }
of_node_put(np);
ret = of_platform_populate(fw_np, s10_of_match, NULL, NULL);
+ of_node_put(fw_np);
if (ret)
return ret;
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/4] fpga: dfl: afu: Pass the correct device to dma_mapping_error()
2019-05-09 21:08 [PATCH 0/4] patches for FPGA Alan Tull
2019-05-09 21:08 ` [PATCH 1/4] fpga: stratix10-soc: fix use-after-free on s10_init() Alan Tull
@ 2019-05-09 21:08 ` Alan Tull
2019-05-09 21:08 ` [PATCH 3/4] fpga: dfl: Add lockdep classes for pdata->lock Alan Tull
2019-05-09 21:08 ` [PATCH 4/4] fpga: dfl: expand minor range when registering chrdev region Alan Tull
3 siblings, 0 replies; 5+ messages in thread
From: Alan Tull @ 2019-05-09 21:08 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Moritz Fischer, Alan Tull, linux-kernel, linux-fpga, Scott Wood,
Wu Hao
From: Scott Wood <swood@redhat.com>
dma_mapping_error() was being called on a different device struct than
what was passed to map/unmap. Besides rendering the error checking
ineffective, it caused a debug splat with CONFIG_DMA_API_DEBUG.
Signed-off-by: Scott Wood <swood@redhat.com>
Acked-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
Acked-by: Alan Tull <atull@kernel.org>
---
drivers/fpga/dfl-afu-dma-region.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
index 0bbc7142f1dc..f7d268f45df0 100644
--- a/drivers/fpga/dfl-afu-dma-region.c
+++ b/drivers/fpga/dfl-afu-dma-region.c
@@ -391,7 +391,7 @@ int afu_dma_map_region(struct dfl_feature_platform_data *pdata,
region->pages[0], 0,
region->length,
DMA_BIDIRECTIONAL);
- if (dma_mapping_error(&pdata->dev->dev, region->iova)) {
+ if (dma_mapping_error(dfl_fpga_pdata_to_parent(pdata), region->iova)) {
dev_err(&pdata->dev->dev, "failed to map for dma\n");
ret = -EFAULT;
goto unpin_pages;
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/4] fpga: dfl: Add lockdep classes for pdata->lock
2019-05-09 21:08 [PATCH 0/4] patches for FPGA Alan Tull
2019-05-09 21:08 ` [PATCH 1/4] fpga: stratix10-soc: fix use-after-free on s10_init() Alan Tull
2019-05-09 21:08 ` [PATCH 2/4] fpga: dfl: afu: Pass the correct device to dma_mapping_error() Alan Tull
@ 2019-05-09 21:08 ` Alan Tull
2019-05-09 21:08 ` [PATCH 4/4] fpga: dfl: expand minor range when registering chrdev region Alan Tull
3 siblings, 0 replies; 5+ messages in thread
From: Alan Tull @ 2019-05-09 21:08 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Moritz Fischer, Alan Tull, linux-kernel, linux-fpga, Scott Wood,
Wu Hao
From: Scott Wood <swood@redhat.com>
struct dfl_feature_platform_data (and it's mutex) is used
by both fme and port devices, and when lockdep is enabled it
complains about nesting between these locks. Tell lockdep about
the difference so it can track each class separately.
Here's the lockdep complaint:
[ 409.680668] WARNING: possible recursive locking detected
[ 409.685983] 5.1.0-rc3.fpga+ #1 Tainted: G E
[ 409.691469] --------------------------------------------
[ 409.696779] fpgaconf/9348 is trying to acquire lock:
[ 409.701746] 00000000a443fe2e (&pdata->lock){+.+.}, at: port_enable_set+0x24/0x60 [dfl_afu]
[ 409.710006]
[ 409.710006] but task is already holding lock:
[ 409.715837] 0000000063b78782 (&pdata->lock){+.+.}, at: fme_pr_ioctl+0x21d/0x330 [dfl_fme]
[ 409.724012]
[ 409.724012] other info that might help us debug this:
[ 409.730535] Possible unsafe locking scenario:
[ 409.730535]
[ 409.736457] CPU0
[ 409.738910] ----
[ 409.741360] lock(&pdata->lock);
[ 409.744679] lock(&pdata->lock);
[ 409.747999]
[ 409.747999] *** DEADLOCK ***
[ 409.747999]
[ 409.753920] May be due to missing lock nesting notation
[ 409.753920]
[ 409.760704] 4 locks held by fpgaconf/9348:
[ 409.764805] #0: 0000000063b78782 (&pdata->lock){+.+.}, at: fme_pr_ioctl+0x21d/0x330 [dfl_fme]
[ 409.773408] #1: 00000000213c8a66 (®ion->mutex){+.+.}, at: fpga_region_program_fpga+0x24/0x200 [fpga_region]
[ 409.783489] #2: 00000000fe63afb9 (&mgr->ref_mutex){+.+.}, at: fpga_mgr_lock+0x15/0x40 [fpga_mgr]
[ 409.792354] #3: 000000000b2285c5 (&bridge->mutex){+.+.}, at: __fpga_bridge_get+0x26/0xa0 [fpga_bridge]
[ 409.801740]
[ 409.801740] stack backtrace:
[ 409.806102] CPU: 45 PID: 9348 Comm: fpgaconf Kdump: loaded Tainted: G E 5.1.0-rc3.fpga+ #1
[ 409.815658] Hardware name: Intel Corporation S2600BT/S2600BT, BIOS SE5C620.86B.01.00.0763.022420181017 02/24/2018
[ 409.825911] Call Trace:
[ 409.828369] dump_stack+0x5e/0x8b
[ 409.831686] __lock_acquire+0xf3d/0x10e0
[ 409.835612] ? find_held_lock+0x3c/0xa0
[ 409.839451] lock_acquire+0xbc/0x1d0
[ 409.843030] ? port_enable_set+0x24/0x60 [dfl_afu]
[ 409.847823] ? port_enable_set+0x24/0x60 [dfl_afu]
[ 409.852616] __mutex_lock+0x86/0x970
[ 409.856195] ? port_enable_set+0x24/0x60 [dfl_afu]
[ 409.860989] ? port_enable_set+0x24/0x60 [dfl_afu]
[ 409.865777] ? __mutex_unlock_slowpath+0x4b/0x290
[ 409.870486] port_enable_set+0x24/0x60 [dfl_afu]
[ 409.875106] fpga_bridges_disable+0x36/0x50 [fpga_bridge]
[ 409.880502] fpga_region_program_fpga+0xea/0x200 [fpga_region]
[ 409.886338] fme_pr_ioctl+0x13e/0x330 [dfl_fme]
[ 409.890870] fme_ioctl+0x66/0xe0 [dfl_fme]
[ 409.894973] do_vfs_ioctl+0xa9/0x720
[ 409.898548] ? lockdep_hardirqs_on+0xf0/0x1a0
[ 409.902907] ksys_ioctl+0x60/0x90
[ 409.906225] __x64_sys_ioctl+0x16/0x20
[ 409.909981] do_syscall_64+0x5a/0x220
[ 409.913644] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 409.918698] RIP: 0033:0x7f9d31b9b8d7
[ 409.922276] Code: 44 00 00 48 8b 05 b9 15 2d 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 89 15 2d 00 f7 d8 64 89 01 48
[ 409.941020] RSP: 002b:00007ffe4cae0d68 EFLAGS: 00000202 ORIG_RAX: 0000000000000010
[ 409.948588] RAX: ffffffffffffffda RBX: 00007f9d32ade6a0 RCX: 00007f9d31b9b8d7
[ 409.955719] RDX: 00007ffe4cae0df0 RSI: 000000000000b680 RDI: 0000000000000003
[ 409.962852] RBP: 0000000000000003 R08: 00007f9d2b70a177 R09: 00007ffe4cae0e40
[ 409.969984] R10: 00007ffe4cae0160 R11: 0000000000000202 R12: 00007ffe4cae0df0
[ 409.977115] R13: 000000000000b680 R14: 0000000000000000 R15: 00007ffe4cae0f60
Signed-off-by: Scott Wood <swood@redhat.com>
Acked-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
---
drivers/fpga/dfl.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index 2c09e502e721..c25217cde5ca 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -40,6 +40,13 @@ enum dfl_fpga_devt_type {
DFL_FPGA_DEVT_MAX,
};
+static struct lock_class_key dfl_pdata_keys[DFL_ID_MAX];
+
+static const char *dfl_pdata_key_strings[DFL_ID_MAX] = {
+ "dfl-fme-pdata",
+ "dfl-port-pdata",
+};
+
/**
* dfl_dev_info - dfl feature device information.
* @name: name string of the feature platform device.
@@ -443,11 +450,16 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
struct platform_device *fdev = binfo->feature_dev;
struct dfl_feature_platform_data *pdata;
struct dfl_feature_info *finfo, *p;
+ enum dfl_id_type type;
int ret, index = 0;
if (!fdev)
return 0;
+ type = feature_dev_id_type(fdev);
+ if (WARN_ON_ONCE(type >= DFL_ID_MAX))
+ return -EINVAL;
+
/*
* we do not need to care for the memory which is associated with
* the platform device. After calling platform_device_unregister(),
@@ -463,6 +475,8 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
pdata->num = binfo->feature_num;
pdata->dfl_cdev = binfo->cdev;
mutex_init(&pdata->lock);
+ lockdep_set_class_and_name(&pdata->lock, &dfl_pdata_keys[type],
+ dfl_pdata_key_strings[type]);
/*
* the count should be initialized to 0 to make sure
@@ -497,7 +511,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
ret = platform_device_add(binfo->feature_dev);
if (!ret) {
- if (feature_dev_id_type(binfo->feature_dev) == PORT_ID)
+ if (type == PORT_ID)
dfl_fpga_cdev_add_port_dev(binfo->cdev,
binfo->feature_dev);
else
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 4/4] fpga: dfl: expand minor range when registering chrdev region
2019-05-09 21:08 [PATCH 0/4] patches for FPGA Alan Tull
` (2 preceding siblings ...)
2019-05-09 21:08 ` [PATCH 3/4] fpga: dfl: Add lockdep classes for pdata->lock Alan Tull
@ 2019-05-09 21:08 ` Alan Tull
3 siblings, 0 replies; 5+ messages in thread
From: Alan Tull @ 2019-05-09 21:08 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Moritz Fischer, Alan Tull, linux-kernel, linux-fpga,
Chengguang Xu, Wu Hao
From: Chengguang Xu <cgxu519@gmx.com>
Actually, total amount of available minor number
for a single major is MINORMASK + 1. So expand
minor range when registering chrdev region.
Signed-off-by: Chengguang Xu <cgxu519@gmx.com>
Acked-by: Wu Hao <hao.wu@intel.com>
Acked-by: Alan Tull <atull@kernel.org>
---
drivers/fpga/dfl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
index c25217cde5ca..4b66aaa32b5a 100644
--- a/drivers/fpga/dfl.c
+++ b/drivers/fpga/dfl.c
@@ -322,7 +322,7 @@ static void dfl_chardev_uinit(void)
for (i = 0; i < DFL_FPGA_DEVT_MAX; i++)
if (MAJOR(dfl_chrdevs[i].devt)) {
unregister_chrdev_region(dfl_chrdevs[i].devt,
- MINORMASK);
+ MINORMASK + 1);
dfl_chrdevs[i].devt = MKDEV(0, 0);
}
}
@@ -332,8 +332,8 @@ static int dfl_chardev_init(void)
int i, ret;
for (i = 0; i < DFL_FPGA_DEVT_MAX; i++) {
- ret = alloc_chrdev_region(&dfl_chrdevs[i].devt, 0, MINORMASK,
- dfl_chrdevs[i].name);
+ ret = alloc_chrdev_region(&dfl_chrdevs[i].devt, 0,
+ MINORMASK + 1, dfl_chrdevs[i].name);
if (ret)
goto exit;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 5+ messages in thread