Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

6 participants
3124 discussions

[PATCH AUTOSEL 6.0 7/7] drm/amdkfd: Fix double release compute pasid

by Sasha Levin

From: Philip Yang <Philip.Yang(a)amd.com> [ Upstream commit 1a799c4c190ea9f0e81028e3eb3037ed0ab17ff5 ] If kfd_process_device_init_vm returns failure after vm is converted to compute vm and vm->pasid set to compute pasid, KFD will not take pdd->drm_file reference. As a result, drm close file handler maybe called to release the compute pasid before KFD process destroy worker to release the same pasid and set vm->pasid to zero, this generates below WARNING backtrace and NULL pointer access. Add helper amdgpu_amdkfd_gpuvm_set_vm_pasid and call it at the last step of kfd_process_device_init_vm, to ensure vm pasid is the original pasid if acquiring vm failed or is the compute pasid with pdd->drm_file reference taken to avoid double release same pasid. amdgpu: Failed to create process VM object ida_free called for id=32770 which is not allocated. WARNING: CPU: 57 PID: 72542 at ../lib/idr.c:522 ida_free+0x96/0x140 RIP: 0010:ida_free+0x96/0x140 Call Trace: amdgpu_pasid_free_delayed+0xe1/0x2a0 [amdgpu] amdgpu_driver_postclose_kms+0x2d8/0x340 [amdgpu] drm_file_free.part.13+0x216/0x270 [drm] drm_close_helper.isra.14+0x60/0x70 [drm] drm_release+0x6e/0xf0 [drm] __fput+0xcc/0x280 ____fput+0xe/0x20 task_work_run+0x96/0xc0 do_exit+0x3d0/0xc10 BUG: kernel NULL pointer dereference, address: 0000000000000000 RIP: 0010:ida_free+0x76/0x140 Call Trace: amdgpu_pasid_free_delayed+0xe1/0x2a0 [amdgpu] amdgpu_driver_postclose_kms+0x2d8/0x340 [amdgpu] drm_file_free.part.13+0x216/0x270 [drm] drm_close_helper.isra.14+0x60/0x70 [drm] drm_release+0x6e/0xf0 [drm] __fput+0xcc/0x280 ____fput+0xe/0x20 task_work_run+0x96/0xc0 do_exit+0x3d0/0xc10 Signed-off-by: Philip Yang <Philip.Yang(a)amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 4 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 39 +++++++++++++------ drivers/gpu/drm/amd/amdkfd/kfd_process.c | 12 ++++-- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h index 647220a8762d..30f145dc8724 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h @@ -265,8 +265,10 @@ int amdgpu_amdkfd_get_pcie_bandwidth_mbytes(struct amdgpu_device *adev, bool is_ (&((struct amdgpu_fpriv *) \ ((struct drm_file *)(drm_priv))->driver_priv)->vm) +int amdgpu_amdkfd_gpuvm_set_vm_pasid(struct amdgpu_device *adev, + struct file *filp, u32 pasid); int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, - struct file *filp, u32 pasid, + struct file *filp, void **process_info, struct dma_fence **ef); void amdgpu_amdkfd_gpuvm_release_process_vm(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 5e184952ec98..f86a132bb761 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1471,10 +1471,9 @@ static void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo) amdgpu_bo_unreserve(bo); } -int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, - struct file *filp, u32 pasid, - void **process_info, - struct dma_fence **ef) +int amdgpu_amdkfd_gpuvm_set_vm_pasid(struct amdgpu_device *adev, + struct file *filp, u32 pasid) + { struct amdgpu_fpriv *drv_priv; struct amdgpu_vm *avm; @@ -1485,10 +1484,6 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, return ret; avm = &drv_priv->vm; - /* Already a compute VM? */ - if (avm->process_info) - return -EINVAL; - /* Free the original amdgpu allocated pasid, * will be replaced with kfd allocated pasid. */ @@ -1497,14 +1492,36 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, amdgpu_vm_set_pasid(adev, avm, 0); } - /* Convert VM into a compute VM */ - ret = amdgpu_vm_make_compute(adev, avm); + ret = amdgpu_vm_set_pasid(adev, avm, pasid); if (ret) return ret; - ret = amdgpu_vm_set_pasid(adev, avm, pasid); + return 0; +} + +int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, + struct file *filp, + void **process_info, + struct dma_fence **ef) +{ + struct amdgpu_fpriv *drv_priv; + struct amdgpu_vm *avm; + int ret; + + ret = amdgpu_file_to_fpriv(filp, &drv_priv); if (ret) return ret; + avm = &drv_priv->vm; + + /* Already a compute VM? */ + if (avm->process_info) + return -EINVAL; + + /* Convert VM into a compute VM */ + ret = amdgpu_vm_make_compute(adev, avm); + if (ret) + return ret; + /* Initialize KFD part of the VM and process info */ ret = init_kfd_vm(avm, process_info, ef); if (ret) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c index 04678f9e214b..febf0e9f7af1 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c @@ -1581,9 +1581,9 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd, p = pdd->process; dev = pdd->dev; - ret = amdgpu_amdkfd_gpuvm_acquire_process_vm( - dev->adev, drm_file, p->pasid, - &p->kgd_process_info, &p->ef); + ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(dev->adev, drm_file, + &p->kgd_process_info, + &p->ef); if (ret) { pr_err("Failed to create process VM object\n"); return ret; @@ -1598,10 +1598,16 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd, if (ret) goto err_init_cwsr; + ret = amdgpu_amdkfd_gpuvm_set_vm_pasid(dev->adev, drm_file, p->pasid); + if (ret) + goto err_set_pasid; + pdd->drm_file = drm_file; return 0; +err_set_pasid: + kfd_process_device_destroy_cwsr_dgpu(pdd); err_init_cwsr: kfd_process_device_destroy_ib_mem(pdd); err_reserve_ib_mem: -- 2.35.1

2 years, 10 months

[PATCH AUTOSEL 6.1 7/7] drm/amdkfd: Fix double release compute pasid

by Sasha Levin

From: Philip Yang <Philip.Yang(a)amd.com> [ Upstream commit 1a799c4c190ea9f0e81028e3eb3037ed0ab17ff5 ] If kfd_process_device_init_vm returns failure after vm is converted to compute vm and vm->pasid set to compute pasid, KFD will not take pdd->drm_file reference. As a result, drm close file handler maybe called to release the compute pasid before KFD process destroy worker to release the same pasid and set vm->pasid to zero, this generates below WARNING backtrace and NULL pointer access. Add helper amdgpu_amdkfd_gpuvm_set_vm_pasid and call it at the last step of kfd_process_device_init_vm, to ensure vm pasid is the original pasid if acquiring vm failed or is the compute pasid with pdd->drm_file reference taken to avoid double release same pasid. amdgpu: Failed to create process VM object ida_free called for id=32770 which is not allocated. WARNING: CPU: 57 PID: 72542 at ../lib/idr.c:522 ida_free+0x96/0x140 RIP: 0010:ida_free+0x96/0x140 Call Trace: amdgpu_pasid_free_delayed+0xe1/0x2a0 [amdgpu] amdgpu_driver_postclose_kms+0x2d8/0x340 [amdgpu] drm_file_free.part.13+0x216/0x270 [drm] drm_close_helper.isra.14+0x60/0x70 [drm] drm_release+0x6e/0xf0 [drm] __fput+0xcc/0x280 ____fput+0xe/0x20 task_work_run+0x96/0xc0 do_exit+0x3d0/0xc10 BUG: kernel NULL pointer dereference, address: 0000000000000000 RIP: 0010:ida_free+0x76/0x140 Call Trace: amdgpu_pasid_free_delayed+0xe1/0x2a0 [amdgpu] amdgpu_driver_postclose_kms+0x2d8/0x340 [amdgpu] drm_file_free.part.13+0x216/0x270 [drm] drm_close_helper.isra.14+0x60/0x70 [drm] drm_release+0x6e/0xf0 [drm] __fput+0xcc/0x280 ____fput+0xe/0x20 task_work_run+0x96/0xc0 do_exit+0x3d0/0xc10 Signed-off-by: Philip Yang <Philip.Yang(a)amd.com> Reviewed-by: Felix Kuehling <Felix.Kuehling(a)amd.com> Signed-off-by: Alex Deucher <alexander.deucher(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 4 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 39 +++++++++++++------ drivers/gpu/drm/amd/amdkfd/kfd_process.c | 12 ++++-- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h index 647220a8762d..30f145dc8724 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h @@ -265,8 +265,10 @@ int amdgpu_amdkfd_get_pcie_bandwidth_mbytes(struct amdgpu_device *adev, bool is_ (&((struct amdgpu_fpriv *) \ ((struct drm_file *)(drm_priv))->driver_priv)->vm) +int amdgpu_amdkfd_gpuvm_set_vm_pasid(struct amdgpu_device *adev, + struct file *filp, u32 pasid); int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, - struct file *filp, u32 pasid, + struct file *filp, void **process_info, struct dma_fence **ef); void amdgpu_amdkfd_gpuvm_release_process_vm(struct amdgpu_device *adev, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 1f76e27f1a35..18a7cbbd00ff 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1473,10 +1473,9 @@ static void amdgpu_amdkfd_gpuvm_unpin_bo(struct amdgpu_bo *bo) amdgpu_bo_unreserve(bo); } -int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, - struct file *filp, u32 pasid, - void **process_info, - struct dma_fence **ef) +int amdgpu_amdkfd_gpuvm_set_vm_pasid(struct amdgpu_device *adev, + struct file *filp, u32 pasid) + { struct amdgpu_fpriv *drv_priv; struct amdgpu_vm *avm; @@ -1487,10 +1486,6 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, return ret; avm = &drv_priv->vm; - /* Already a compute VM? */ - if (avm->process_info) - return -EINVAL; - /* Free the original amdgpu allocated pasid, * will be replaced with kfd allocated pasid. */ @@ -1499,14 +1494,36 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, amdgpu_vm_set_pasid(adev, avm, 0); } - /* Convert VM into a compute VM */ - ret = amdgpu_vm_make_compute(adev, avm); + ret = amdgpu_vm_set_pasid(adev, avm, pasid); if (ret) return ret; - ret = amdgpu_vm_set_pasid(adev, avm, pasid); + return 0; +} + +int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct amdgpu_device *adev, + struct file *filp, + void **process_info, + struct dma_fence **ef) +{ + struct amdgpu_fpriv *drv_priv; + struct amdgpu_vm *avm; + int ret; + + ret = amdgpu_file_to_fpriv(filp, &drv_priv); if (ret) return ret; + avm = &drv_priv->vm; + + /* Already a compute VM? */ + if (avm->process_info) + return -EINVAL; + + /* Convert VM into a compute VM */ + ret = amdgpu_vm_make_compute(adev, avm); + if (ret) + return ret; + /* Initialize KFD part of the VM and process info */ ret = init_kfd_vm(avm, process_info, ef); if (ret) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c index 9821fa9268d3..dd351105c1bc 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c @@ -1576,9 +1576,9 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd, p = pdd->process; dev = pdd->dev; - ret = amdgpu_amdkfd_gpuvm_acquire_process_vm( - dev->adev, drm_file, p->pasid, - &p->kgd_process_info, &p->ef); + ret = amdgpu_amdkfd_gpuvm_acquire_process_vm(dev->adev, drm_file, + &p->kgd_process_info, + &p->ef); if (ret) { pr_err("Failed to create process VM object\n"); return ret; @@ -1593,10 +1593,16 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd, if (ret) goto err_init_cwsr; + ret = amdgpu_amdkfd_gpuvm_set_vm_pasid(dev->adev, drm_file, p->pasid); + if (ret) + goto err_set_pasid; + pdd->drm_file = drm_file; return 0; +err_set_pasid: + kfd_process_device_destroy_cwsr_dgpu(pdd); err_init_cwsr: kfd_process_device_destroy_ib_mem(pdd); err_reserve_ib_mem: -- 2.35.1

2 years, 10 months

Re: [PATCH v3] usb: gadget: aspeed_udc: Add check for dma_alloc_coherent

by Jiasheng Jiang

On Fri, Dec 23, 2022 at 10:54:37PM +0800, Greg KH wrote: >> diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c >> index 01968e2167f9..7dc2457c7460 100644 >> --- a/drivers/usb/gadget/udc/aspeed_udc.c >> +++ b/drivers/usb/gadget/udc/aspeed_udc.c >> @@ -1516,6 +1516,10 @@ static int ast_udc_probe(struct platform_device *pdev) >> AST_UDC_EP_DMA_SIZE * >> AST_UDC_NUM_ENDPOINTS, >> &udc->ep0_buf_dma, GFP_KERNEL); >> + if (!udc->ep0_buf) { >> + rc = -ENOMEM; >> + goto err; >> + } >> >> udc->gadget.speed = USB_SPEED_UNKNOWN; >> udc->gadget.max_speed = USB_SPEED_HIGH; >> -- >> 2.25.1 >> > > Why is this just a duplicate of the patch previously submitted here: > https://lore.kernel.org/r/20221125092833.74822-1-yuancan@huawei.com > > confused, > > greg k-h Yes, it is the same as mine. As the previous patch had not been merged into the Linux kernel, my tool found the same error and report it. And both of us chose the most concise way to fix the error. That is why the patches are the same. Thanks, Jiang

2 years, 10 months

Re: [PATCH v3] usb: gadget: aspeed_udc: Add check for dma_alloc_coherent

by Jiasheng Jiang

Yes, it is the same as mine. As the previous patch had not been merged into the Linux kernel, my tool found the same error and report it. And both of us chose the most concise way to fix the error. That is why the patches are the same. Thanks, Jiang

2 years, 10 months

[PATCH v3] usb: gadget: aspeed_udc: Add check for dma_alloc_coherent

by Jiasheng Jiang

Add the check for the return value of dma_alloc_coherent in order to avoid NULL pointer dereference. This flaw was found using an experimental static analysis tool we are developing, APP-Miner, which has not been disclosed. The allyesconfig build using GCC 9.3.0 shows no new warning. As we don't have a UDC device to test with, no runtime testing was able to be performed. Signed-off-by: Jiasheng Jiang <jiasheng(a)iscas.ac.cn> --- Changelog: v2 -> v3: 1. Add information of finding tool and tests to commit message. v1 -> v2: 1. Add "goto err;" when allocation fails. --- drivers/usb/gadget/udc/aspeed_udc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c index 01968e2167f9..7dc2457c7460 100644 --- a/drivers/usb/gadget/udc/aspeed_udc.c +++ b/drivers/usb/gadget/udc/aspeed_udc.c @@ -1516,6 +1516,10 @@ static int ast_udc_probe(struct platform_device *pdev) AST_UDC_EP_DMA_SIZE * AST_UDC_NUM_ENDPOINTS, &udc->ep0_buf_dma, GFP_KERNEL); + if (!udc->ep0_buf) { + rc = -ENOMEM; + goto err; + } udc->gadget.speed = USB_SPEED_UNKNOWN; udc->gadget.max_speed = USB_SPEED_HIGH; -- 2.25.1

2 years, 10 months

[PATCH v2] usb: gadget: aspeed_udc: Add check for dma_alloc_coherent

by Jiasheng Jiang

Add the check for the return value of dma_alloc_coherent in order to avoid NULL pointer dereference. This flaw was found using an experimental static analysis tool we are developing, APP-Miner, which has not been disclosed. The allyesconfig build using GCC 9.3.0 shows no new warning. As we don't have a UDC device to test with, no runtime testing was able to be performed. Signed-off-by: Jiasheng Jiang <jiasheng(a)iscas.ac.cn> --- Changelog: v1 -> v2: 1. Add "goto err;" when allocation fails. --- drivers/usb/gadget/udc/aspeed_udc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c index 01968e2167f9..7dc2457c7460 100644 --- a/drivers/usb/gadget/udc/aspeed_udc.c +++ b/drivers/usb/gadget/udc/aspeed_udc.c @@ -1516,6 +1516,10 @@ static int ast_udc_probe(struct platform_device *pdev) AST_UDC_EP_DMA_SIZE * AST_UDC_NUM_ENDPOINTS, &udc->ep0_buf_dma, GFP_KERNEL); + if (!udc->ep0_buf) { + rc = -ENOMEM; + goto err; + } udc->gadget.speed = USB_SPEED_UNKNOWN; udc->gadget.max_speed = USB_SPEED_HIGH; -- 2.25.1

2 years, 11 months

[PATCH v2] net: ksz884x: Remove some unused functions

by Jiapeng Chong

These functions are defined in the ksz884x.c file, but not called elsewhere, so delete these unused functions. drivers/net/ethernet/micrel/ksz884x.c:2212:20: warning: unused function 'port_cfg_force_flow_ctrl'. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=3418 Reported-by: Abaci Robot <abaci(a)linux.alibaba.com> Signed-off-by: Jiapeng Chong <jiapeng.chong(a)linux.alibaba.com> --- Changes in v2: -Delete more unused functions. drivers/net/ethernet/micrel/ksz884x.c | 89 --------------------------- 1 file changed, 89 deletions(-) diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c index e6acd1e7b263..5d6ed7a63e59 100644 --- a/drivers/net/ethernet/micrel/ksz884x.c +++ b/drivers/net/ethernet/micrel/ksz884x.c @@ -2209,102 +2209,13 @@ static inline void port_cfg_back_pressure(struct ksz_hw *hw, int p, int set) KS8842_PORT_CTRL_2_OFFSET, PORT_BACK_PRESSURE, set); } -static inline void port_cfg_force_flow_ctrl(struct ksz_hw *hw, int p, int set) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_FORCE_FLOW_CTRL, set); -} - -static inline int port_chk_back_pressure(struct ksz_hw *hw, int p) -{ - return port_chk(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_BACK_PRESSURE); -} - -static inline int port_chk_force_flow_ctrl(struct ksz_hw *hw, int p) -{ - return port_chk(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_FORCE_FLOW_CTRL); -} - /* Spanning Tree */ -static inline void port_cfg_rx(struct ksz_hw *hw, int p, int set) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_RX_ENABLE, set); -} - -static inline void port_cfg_tx(struct ksz_hw *hw, int p, int set) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_TX_ENABLE, set); -} - static inline void sw_cfg_fast_aging(struct ksz_hw *hw, int set) { sw_cfg(hw, KS8842_SWITCH_CTRL_1_OFFSET, SWITCH_FAST_AGING, set); } -static inline void sw_flush_dyn_mac_table(struct ksz_hw *hw) -{ - if (!(hw->overrides & FAST_AGING)) { - sw_cfg_fast_aging(hw, 1); - mdelay(1); - sw_cfg_fast_aging(hw, 0); - } -} - -/* VLAN */ - -static inline void port_cfg_ins_tag(struct ksz_hw *hw, int p, int insert) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_1_OFFSET, PORT_INSERT_TAG, insert); -} - -static inline void port_cfg_rmv_tag(struct ksz_hw *hw, int p, int remove) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_1_OFFSET, PORT_REMOVE_TAG, remove); -} - -static inline int port_chk_ins_tag(struct ksz_hw *hw, int p) -{ - return port_chk(hw, p, - KS8842_PORT_CTRL_1_OFFSET, PORT_INSERT_TAG); -} - -static inline int port_chk_rmv_tag(struct ksz_hw *hw, int p) -{ - return port_chk(hw, p, - KS8842_PORT_CTRL_1_OFFSET, PORT_REMOVE_TAG); -} - -static inline void port_cfg_dis_non_vid(struct ksz_hw *hw, int p, int set) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_DISCARD_NON_VID, set); -} - -static inline void port_cfg_in_filter(struct ksz_hw *hw, int p, int set) -{ - port_cfg(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_INGRESS_VLAN_FILTER, set); -} - -static inline int port_chk_dis_non_vid(struct ksz_hw *hw, int p) -{ - return port_chk(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_DISCARD_NON_VID); -} - -static inline int port_chk_in_filter(struct ksz_hw *hw, int p) -{ - return port_chk(hw, p, - KS8842_PORT_CTRL_2_OFFSET, PORT_INGRESS_VLAN_FILTER); -} - /* Mirroring */ static inline void port_cfg_mirror_sniffer(struct ksz_hw *hw, int p, int set) -- 2.20.1.7.g153144c

2 years, 11 months

[PATCH v2] usb: gadget: aspeed_udc: Add check for dma_alloc_coherent

by Jiasheng Jiang

Add the check for the return value of dma_alloc_coherent in order to avoid NULL pointer dereference. Fixes: 055276c13205 ("usb: gadget: add Aspeed ast2600 udc driver") Signed-off-by: Jiasheng Jiang <jiasheng(a)iscas.ac.cn> --- Changelog: v1 -> v2: 1. Add "goto err;" when allocation fails. --- drivers/usb/gadget/udc/aspeed_udc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c index 01968e2167f9..7dc2457c7460 100644 --- a/drivers/usb/gadget/udc/aspeed_udc.c +++ b/drivers/usb/gadget/udc/aspeed_udc.c @@ -1516,6 +1516,10 @@ static int ast_udc_probe(struct platform_device *pdev) AST_UDC_EP_DMA_SIZE * AST_UDC_NUM_ENDPOINTS, &udc->ep0_buf_dma, GFP_KERNEL); + if (!udc->ep0_buf) { + rc = -ENOMEM; + goto err; + } udc->gadget.speed = USB_SPEED_UNKNOWN; udc->gadget.max_speed = USB_SPEED_HIGH; -- 2.25.1

2 years, 11 months

Re: [PATCH] usb: gadget: aspeed_udc: Add check for dma_alloc_coherent

by Jiasheng Jiang

Thanks, I found my mistake and I will submit a v2. > And how did you find this potential problem? What tool did you use and > why did you not follow the documentation for properly describing the > tool? I used a tool I wrote myself to find it, which is unpublished. Therefore, I think it is okay to submit patches without description of the tools. Thanks, Jiang

2 years, 11 months

[PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs

by Rijo Thomas

For AMD Secure Processor (ASP) to map and access TEE ring buffer, the ring buffer address sent by host to ASP must be a real physical address and the pages must be physically contiguous. In a virtualized environment though, when the driver is running in a guest VM, the pages allocated by __get_free_pages() may not be contiguous in the host (or machine) physical address space. Guests will see a guest (or pseudo) physical address and not the actual host (or machine) physical address. The TEE running on ASP cannot decipher pseudo physical addresses. It needs host or machine physical address. To resolve this problem, use DMA APIs for allocating buffers that must be shared with TEE. This will ensure that the pages are contiguous in host (or machine) address space. If the DMA handle is an IOVA, translate it into a physical address before sending it to ASP. This patch also exports two APIs (one for buffer allocation and another to free the buffer). This API can be used by AMD-TEE driver to share buffers with TEE. Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge") Cc: Tom Lendacky <thomas.lendacky(a)amd.com> Cc: stable(a)vger.kernel.org Signed-off-by: Rijo Thomas <Rijo-john.Thomas(a)amd.com> Co-developed-by: Jeshwanth <JESHWANTHKUMAR.NK(a)amd.com> Signed-off-by: Jeshwanth <JESHWANTHKUMAR.NK(a)amd.com> Reviewed-by: Devaraj Rangasamy <Devaraj.Rangasamy(a)amd.com> --- drivers/crypto/ccp/psp-dev.c | 6 +- drivers/crypto/ccp/tee-dev.c | 116 ++++++++++++++++++++++------------- drivers/crypto/ccp/tee-dev.h | 9 +-- include/linux/psp-tee.h | 47 ++++++++++++++ 4 files changed, 127 insertions(+), 51 deletions(-) diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c index c9c741ac8442..2b86158d7435 100644 --- a/drivers/crypto/ccp/psp-dev.c +++ b/drivers/crypto/ccp/psp-dev.c @@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp) goto e_err; } + if (sp->set_psp_master_device) + sp->set_psp_master_device(sp); + ret = psp_init(psp); if (ret) goto e_irq; - if (sp->set_psp_master_device) - sp->set_psp_master_device(sp); - /* Enable interrupt */ iowrite32(-1, psp->io_regs + psp->vdata->inten_reg); diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c index 5c9d47f3be37..1631d9851e54 100644 --- a/drivers/crypto/ccp/tee-dev.c +++ b/drivers/crypto/ccp/tee-dev.c @@ -12,8 +12,9 @@ #include <linux/mutex.h> #include <linux/delay.h> #include <linux/slab.h> +#include <linux/dma-direct.h> +#include <linux/iommu.h> #include <linux/gfp.h> -#include <linux/psp-sev.h> #include <linux/psp-tee.h> #include "psp-dev.h" @@ -21,25 +22,64 @@ static bool psp_dead; +struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp) +{ + struct psp_device *psp = psp_get_master_device(); + struct dma_buffer *dma_buf; + struct iommu_domain *dom; + + if (!psp || !size) + return NULL; + + dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL); + if (!dma_buf) + return NULL; + + dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, &dma_buf->dma, gfp); + if (!dma_buf->vaddr || !dma_buf->dma) { + kfree(dma_buf); + return NULL; + } + + dma_buf->size = size; + + dom = iommu_get_domain_for_dev(psp->dev); + if (dom) + dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma); + else + dma_buf->paddr = dma_buf->dma; + + return dma_buf; +} +EXPORT_SYMBOL(psp_tee_alloc_dmabuf); + +void psp_tee_free_dmabuf(struct dma_buffer *dma_buf) +{ + struct psp_device *psp = psp_get_master_device(); + + if (!psp || !dma_buf) + return; + + dma_free_coherent(psp->dev, dma_buf->size, + dma_buf->vaddr, dma_buf->dma); + + kfree(dma_buf); +} +EXPORT_SYMBOL(psp_tee_free_dmabuf); + static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size) { struct ring_buf_manager *rb_mgr = &tee->rb_mgr; - void *start_addr; if (!ring_size) return -EINVAL; - /* We need actual physical address instead of DMA address, since - * Trusted OS running on AMD Secure Processor will map this region - */ - start_addr = (void *)__get_free_pages(GFP_KERNEL, get_order(ring_size)); - if (!start_addr) + rb_mgr->ring_buf = psp_tee_alloc_dmabuf(ring_size, + GFP_KERNEL | __GFP_ZERO); + if (!rb_mgr->ring_buf) { + dev_err(tee->dev, "ring allocation failed\n"); return -ENOMEM; - - memset(start_addr, 0x0, ring_size); - rb_mgr->ring_start = start_addr; - rb_mgr->ring_size = ring_size; - rb_mgr->ring_pa = __psp_pa(start_addr); + } mutex_init(&rb_mgr->mutex); return 0; @@ -49,15 +89,8 @@ static void tee_free_ring(struct psp_tee_device *tee) { struct ring_buf_manager *rb_mgr = &tee->rb_mgr; - if (!rb_mgr->ring_start) - return; + psp_tee_free_dmabuf(rb_mgr->ring_buf); - free_pages((unsigned long)rb_mgr->ring_start, - get_order(rb_mgr->ring_size)); - - rb_mgr->ring_start = NULL; - rb_mgr->ring_size = 0; - rb_mgr->ring_pa = 0; mutex_destroy(&rb_mgr->mutex); } @@ -81,35 +114,36 @@ static int tee_wait_cmd_poll(struct psp_tee_device *tee, unsigned int timeout, return -ETIMEDOUT; } -static -struct tee_init_ring_cmd *tee_alloc_cmd_buffer(struct psp_tee_device *tee) +struct dma_buffer *tee_alloc_cmd_buffer(struct psp_tee_device *tee) { struct tee_init_ring_cmd *cmd; + struct dma_buffer *cmd_buffer; - cmd = kzalloc(sizeof(*cmd), GFP_KERNEL); - if (!cmd) + cmd_buffer = psp_tee_alloc_dmabuf(sizeof(*cmd), + GFP_KERNEL | __GFP_ZERO); + if (!cmd_buffer) return NULL; - cmd->hi_addr = upper_32_bits(tee->rb_mgr.ring_pa); - cmd->low_addr = lower_32_bits(tee->rb_mgr.ring_pa); - cmd->size = tee->rb_mgr.ring_size; + cmd = (struct tee_init_ring_cmd *)cmd_buffer->vaddr; + cmd->hi_addr = upper_32_bits(tee->rb_mgr.ring_buf->paddr); + cmd->low_addr = lower_32_bits(tee->rb_mgr.ring_buf->paddr); + cmd->size = tee->rb_mgr.ring_buf->size; dev_dbg(tee->dev, "tee: ring address: high = 0x%x low = 0x%x size = %u\n", cmd->hi_addr, cmd->low_addr, cmd->size); - return cmd; + return cmd_buffer; } -static inline void tee_free_cmd_buffer(struct tee_init_ring_cmd *cmd) +static inline void tee_free_cmd_buffer(struct dma_buffer *cmd_buffer) { - kfree(cmd); + psp_tee_free_dmabuf(cmd_buffer); } static int tee_init_ring(struct psp_tee_device *tee) { int ring_size = MAX_RING_BUFFER_ENTRIES * sizeof(struct tee_ring_cmd); - struct tee_init_ring_cmd *cmd; - phys_addr_t cmd_buffer; + struct dma_buffer *cmd_buffer; unsigned int reg; int ret; @@ -123,21 +157,19 @@ static int tee_init_ring(struct psp_tee_device *tee) tee->rb_mgr.wptr = 0; - cmd = tee_alloc_cmd_buffer(tee); - if (!cmd) { + cmd_buffer = tee_alloc_cmd_buffer(tee); + if (!cmd_buffer) { tee_free_ring(tee); return -ENOMEM; } - cmd_buffer = __psp_pa((void *)cmd); - /* Send command buffer details to Trusted OS by writing to * CPU-PSP message registers */ - iowrite32(lower_32_bits(cmd_buffer), + iowrite32(lower_32_bits(cmd_buffer->paddr), tee->io_regs + tee->vdata->cmdbuff_addr_lo_reg); - iowrite32(upper_32_bits(cmd_buffer), + iowrite32(upper_32_bits(cmd_buffer->paddr), tee->io_regs + tee->vdata->cmdbuff_addr_hi_reg); iowrite32(TEE_RING_INIT_CMD, tee->io_regs + tee->vdata->cmdresp_reg); @@ -157,7 +189,7 @@ static int tee_init_ring(struct psp_tee_device *tee) } free_buf: - tee_free_cmd_buffer(cmd); + tee_free_cmd_buffer(cmd_buffer); return ret; } @@ -167,7 +199,7 @@ static void tee_destroy_ring(struct psp_tee_device *tee) unsigned int reg; int ret; - if (!tee->rb_mgr.ring_start) + if (!tee->rb_mgr.ring_buf->vaddr) return; if (psp_dead) @@ -256,7 +288,7 @@ static int tee_submit_cmd(struct psp_tee_device *tee, enum tee_cmd_id cmd_id, do { /* Get pointer to ring buffer command entry */ cmd = (struct tee_ring_cmd *) - (tee->rb_mgr.ring_start + tee->rb_mgr.wptr); + (tee->rb_mgr.ring_buf->vaddr + tee->rb_mgr.wptr); rptr = ioread32(tee->io_regs + tee->vdata->ring_rptr_reg); @@ -305,7 +337,7 @@ static int tee_submit_cmd(struct psp_tee_device *tee, enum tee_cmd_id cmd_id, /* Update local copy of write pointer */ tee->rb_mgr.wptr += sizeof(struct tee_ring_cmd); - if (tee->rb_mgr.wptr >= tee->rb_mgr.ring_size) + if (tee->rb_mgr.wptr >= tee->rb_mgr.ring_buf->size) tee->rb_mgr.wptr = 0; /* Trigger interrupt to Trusted OS */ diff --git a/drivers/crypto/ccp/tee-dev.h b/drivers/crypto/ccp/tee-dev.h index 49d26158b71e..9238487ee8bf 100644 --- a/drivers/crypto/ccp/tee-dev.h +++ b/drivers/crypto/ccp/tee-dev.h @@ -16,6 +16,7 @@ #include <linux/device.h> #include <linux/mutex.h> +#include <linux/psp-tee.h> #define TEE_DEFAULT_TIMEOUT 10 #define MAX_BUFFER_SIZE 988 @@ -48,17 +49,13 @@ struct tee_init_ring_cmd { /** * struct ring_buf_manager - Helper structure to manage ring buffer. - * @ring_start: starting address of ring buffer - * @ring_size: size of ring buffer in bytes - * @ring_pa: physical address of ring buffer * @wptr: index to the last written entry in ring buffer + * @ring_buf: ring buffer allocated using DMA api */ struct ring_buf_manager { struct mutex mutex; /* synchronizes access to ring buffer */ - void *ring_start; - u32 ring_size; - phys_addr_t ring_pa; u32 wptr; + struct dma_buffer *ring_buf; }; struct psp_tee_device { diff --git a/include/linux/psp-tee.h b/include/linux/psp-tee.h index cb0c95d6d76b..c0fa922f24d4 100644 --- a/include/linux/psp-tee.h +++ b/include/linux/psp-tee.h @@ -13,6 +13,7 @@ #include <linux/types.h> #include <linux/errno.h> +#include <linux/dma-mapping.h> /* This file defines the Trusted Execution Environment (TEE) interface commands * and the API exported by AMD Secure Processor driver to communicate with @@ -40,6 +41,20 @@ enum tee_cmd_id { TEE_CMD_ID_UNMAP_SHARED_MEM, }; +/** + * struct dma_buffer - Structure for a DMA buffer. + * @dma: DMA buffer address + * @paddr: Physical address of DMA buffer + * @vaddr: CPU virtual address of DMA buffer + * @size: Size of DMA buffer in bytes + */ +struct dma_buffer { + dma_addr_t dma; + phys_addr_t paddr; + void *vaddr; + unsigned long size; +}; + #ifdef CONFIG_CRYPTO_DEV_SP_PSP /** * psp_tee_process_cmd() - Process command in Trusted Execution Environment @@ -75,6 +90,28 @@ int psp_tee_process_cmd(enum tee_cmd_id cmd_id, void *buf, size_t len, */ int psp_check_tee_status(void); +/** + * psp_tee_alloc_dmabuf() - Allocates memory of requested size and flags using + * dma_alloc_coherent() API. + * + * This function can be used to allocate a shared memory region between the + * host and PSP TEE. + * + * Returns: + * non-NULL a valid pointer to struct dma_buffer + * NULL on failure + */ +struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp); + +/** + * psp_tee_free_dmabuf() - Deallocates memory using dma_free_coherent() API. + * + * This function can be used to release shared memory region between host + * and PSP TEE. + * + */ +void psp_tee_free_dmabuf(struct dma_buffer *dma_buffer); + #else /* !CONFIG_CRYPTO_DEV_SP_PSP */ static inline int psp_tee_process_cmd(enum tee_cmd_id cmd_id, void *buf, @@ -87,5 +124,15 @@ static inline int psp_check_tee_status(void) { return -ENODEV; } + +static inline +struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp) +{ + return NULL; +} + +static inline void psp_tee_free_dmabuf(struct dma_buffer *dma_buffer) +{ +} #endif /* CONFIG_CRYPTO_DEV_SP_PSP */ #endif /* __PSP_TEE_H_ */ -- 2.25.1

2 years, 11 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig