[PATCH v2] drm/amdkfd: validate pdd where it acquired first
Currently pdd is validate after dereferencing it, which is
not correct, Thus validate pdd before its first use.
Signed-off-by: Maninder Singh
---
v1: remove validation of pdd after its usage
v2: do validation at first place rather than removing
drivers/gpu/drm/amd/amdkfd/kfd_process.c |9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 8a1f999..9be0070 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -420,6 +420,12 @@ void kfd_unbind_process_from_device(struct kfd_dev *dev,
unsigned int pasid)
pqm_uninit(>pqm);
pdd = kfd_get_process_device_data(dev, p);
+
+ if (!pdd) {
+ mutex_unlock(>mutex);
+ return;
+ }
+
if (pdd->reset_wavefronts) {
dbgdev_wave_reset_wavefronts(pdd->dev, p);
pdd->reset_wavefronts = false;
@@ -431,8 +437,7 @@ void kfd_unbind_process_from_device(struct kfd_dev *dev,
unsigned int pasid)
* We don't call amd_iommu_unbind_pasid() here
* because the IOMMU called us.
*/
- if (pdd)
- pdd->bound = false;
+ pdd->bound = false;
mutex_unlock(>mutex);
}
--
1.7.9.5
[RFC][PATCH 1/1] drm/amdkfd: Remove redundant pdd validation
pdd is already dereferenced before this check. So it is redundtant to validate pdd here. Signed-off-by: Maninder Singh --- drivers/gpu/drm/amd/amdkfd/kfd_process.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c index 8a1f999..4dbc4e5 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c @@ -431,8 +431,7 @@ void kfd_unbind_process_from_device(struct kfd_dev *dev, unsigned int pasid) * We don't call amd_iommu_unbind_pasid() here * because the IOMMU called us. */ - if (pdd) - pdd->bound = false; + pdd->bound = false; mutex_unlock(>mutex); } -- 1.7.9.5
[PATCH v2] drm/amdgpu: use kzalloc for allocating one thing
Use kzalloc rather than kcalloc(1.. for allocating
one thing.
Signed-off-by: Maninder Singh
Reviewed-by: Vaneet Narang
Reviewed-by: Christian Konig
---
v2: Chnged shortlog prefix - gpu/amdgpu
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d3706a4..dd3415d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -674,7 +674,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
return 0;
if (gtt && gtt->userptr) {
- ttm->sg = kcalloc(1, sizeof(struct sg_table), GFP_KERNEL);
+ ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
--
1.7.9.5
[PATCH v2] drm/amdgpu: remove unnecessary check before kfree
kfree(NULL) is safe and this check is probably not required Signed-off-by: Maninder Singh Reviewed-by: Vaneet Narang Reviewed-by: Christian Konig --- v2: Chnged shortlog prefix - gpu/amdgpu drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index fec487d..a85cd08 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1575,8 +1575,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_fence_driver_fini(adev); amdgpu_fbdev_fini(adev); r = amdgpu_fini(adev); - if (adev->ip_block_enabled) - kfree(adev->ip_block_enabled); + kfree(adev->ip_block_enabled); adev->ip_block_enabled = NULL; adev->accel_working = false; /* free i2c buses */ -- 1.7.9.5
[PATCH 1/1] gpu/drm: remove unnecessary check before kfree
kfree(NULL) is safe and this check is probably not required Signed-off-by: Maninder Singh Reviewed-by: Vaneet Narang --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index fec487d..a85cd08 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1575,8 +1575,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_fence_driver_fini(adev); amdgpu_fbdev_fini(adev); r = amdgpu_fini(adev); - if (adev->ip_block_enabled) - kfree(adev->ip_block_enabled); + kfree(adev->ip_block_enabled); adev->ip_block_enabled = NULL; adev->accel_working = false; /* free i2c buses */ -- 1.7.9.5
[PATCH 1/1] gpu/drm: use kzalloc for allocating one thing
Use kzalloc rather than kcalloc(1.. for allocating
one thing.
Signed-off-by: Maninder Singh
Reviewed-by: Vaneet Narang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index d3706a4..dd3415d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -674,7 +674,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_tt *ttm)
return 0;
if (gtt && gtt->userptr) {
- ttm->sg = kcalloc(1, sizeof(struct sg_table), GFP_KERNEL);
+ ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
--
1.7.9.5
[PATCH 1/1] gpu/drm: remove unnecessary check before kfree
Hi, >The shortlog prefix of both your patches should be "drm/amdgpu:" instead >of "gpu/drm:". With that fixed, both are Do i need to send V2 of patch with that change or you have fixed that? >Reviewed-by: Michel Dänzer Thanks. ..
[PATCH v2] drm/gma500: mdfld: Remove unncessary check
sender is dereferrenced before NULL check
struct drm_device *dev = sender->dev;
and due to there is warning during static analysis:
warn: variable dereferenced before check 'sender'
__read_panel_data Function is called by mdfld_dsi_read_mcs
and there is a same check, Thus removing the check from
mdfld_dsi_read_mcs and initializing dev struct after NULL check
of sender.
Suggested-by: Patrick Jakobsson
Signed-off-by: Maninder Singh
Reviewed-by: Vaneet Narang
---
v1: removes check from __read_panel_data
v2: remove check from mdfld_dsi_read_mcs instead of __read_panel_data
drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
index 6b43ae3..959aaeb 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
@@ -520,7 +520,7 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender
*sender, u8 data_type,
u8 *data, u16 len, u32 *data_out, u16 len_out, bool hs)
{
unsigned long flags;
- struct drm_device *dev = sender->dev;
+ struct drm_device *dev;
int i;
u32 gen_data_reg;
int retry = MDFLD_DSI_READ_MAX_COUNT;
@@ -530,6 +530,8 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender
*sender, u8 data_type,
return -EINVAL;
}
+ dev = sender->dev;
+
/**
* do reading.
* 0) send out generic read request
@@ -576,11 +578,6 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender
*sender, u8 data_type,
int mdfld_dsi_read_mcs(struct mdfld_dsi_pkg_sender *sender, u8 cmd,
u32 *data, u16 len, bool hs)
{
- if (!sender || !data || !len) {
- DRM_ERROR("Invalid parameters\n");
- return -EINVAL;
- }
-
return __read_panel_data(sender, MIPI_DSI_DCS_READ, , 1,
data, len, hs);
}
--
1.7.9.5
[PATCH 1/1]drm/gma500: mdfld: Remove unncessary code
sender is dereferrenced before NULL check
struct drm_device *dev = sender->dev;
and due to this we get warning during static analysis:
warn: variable dereferenced before check 'sender'
__read_panel_data Function is called by mdfld_dsi_read_mcs
and there is a same check, Thus removing the check from
__read_panel_data.
Signed-off-by: Maninder Singh
Reviewed-by: Vaneet Narang
---
drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c |5 -
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
index 6b43ae3..d29b881 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
@@ -525,11 +525,6 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender
*sender, u8 data_type,
u32 gen_data_reg;
int retry = MDFLD_DSI_READ_MAX_COUNT;
- if (!sender || !data_out || !len_out) {
- DRM_ERROR("Invalid parameters\n");
- return -EINVAL;
- }
-
/**
* do reading.
* 0) send out generic read request
--
1.7.1
[PATCH 1/1]drm/gma500: mdfld: Remove unncessary code
Hi,
>> - if (!sender || !data_out || !len_out) {
>> - DRM_ERROR("Invalid parameters\n");
>> - return -EINVAL;
>> - }
>> -
>
>I would prefer to have these kind of checks where it actually matters
>(ie. in __read_panel_data()). The saner thing would be to move the
>dereference until after the check and remove the duplicated check from
>mdfld_dsi_read_mcs(). That would prevent any further need for adding
>additional checks whenever calling __read_panel_data().
Ok agree, But i am thinking whether this initilaization has to be there?
struct drm_device *dev = sender->dev;
Because in function __read_panel_data I saw no usage of this dev struct,
So along with check from mdfld_dsi_read_mcs, can we remove this dev from
__read_panel_data also ? Or i missed something in code?
Thanks,
Maninder
-
[PATCH 1/1] drm/radeon: use kzalloc for allocating one thing
Use kzalloc for allocating one thing rather than
kcalloc(1...
The semantic patch that makes this change is as follows:
//
@@
@@
- kcalloc(1,
+ kzalloc(
...)
//
Signed-off-by: Maninder Singh
Reviewed-by: Vaneet Narang
---
drivers/gpu/drm/radeon/radeon_ttm.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index edafd3c..06ac59f 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -719,7 +719,7 @@ static int radeon_ttm_tt_populate(struct ttm_tt *ttm)
return 0;
if (gtt && gtt->userptr) {
- ttm->sg = kcalloc(1, sizeof(struct sg_table), GFP_KERNEL);
+ ttm->sg = kzalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!ttm->sg)
return -ENOMEM;
--
1.7.1
[EDT][PATCh 1/1]mdfld_dsi_pkg_sender.c : Fix Possible NULL Pointer dereference
EP-AA9D1F29B02341529D96C06444D8471D
Hi,
There is NULL pointer check for sender after dereferencing sender in
__read_panel_data as below:-
struct drm_device *dev = sender->dev;
...
if (!sender || !data || !len)
And from codeflow
mdfld_dsi_get_panel_status --> mdfld_dsi_read_mcs --> __read_panel_data
In mdfld_dsi_get_panel_status & mdfld_dsi_read_mcs there is already a same
check.
---Cut
if (!sender || !data || !len) {
DRM_ERROR("Invalid parameters\n");
return -EINVAL;
}
return __read_panel_data(sender, MIPI_DSI_DCS_READ, , 1,
data, len, hs);
Cut---
So either we can remove this check from __read_panel_data ,
or if we want to have defensive code then below change should be included.
Subject: [PATCH 1/1] mdfld_dsi_pkg_sender.c : Initialize dev struct after NULL
check of sender
Signed-off-by: Maninder Singh
Reviewed-By: Vaneet Narang
---
drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
index 6b43ae3..6f2b2c9 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_pkg_sender.c
@@ -520,7 +520,7 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender
*sender, u8 data_type,
u8 *data, u16 len, u32 *data_out, u16 len_out, bool hs)
{
unsigned long flags;
- struct drm_device *dev = sender->dev;
+ struct drm_device *dev;
int i;
u32 gen_data_reg;
int retry = MDFLD_DSI_READ_MAX_COUNT;
@@ -530,6 +530,8 @@ static int __read_panel_data(struct mdfld_dsi_pkg_sender
*sender, u8 data_type,
return -EINVAL;
}
+ dev = sender->dev;
+
/**
* do reading.
* 0) send out generic read request
--
1.7.1
Thanks
Maninder
