[v3,4/6] drm/i915/gvt: Check if cur_pt_type is valid

Submitted by Aleksei Gimbitskii on April 23, 2019, 12:04 p.m.

Details

Message ID 20190423120413.30929-5-aleksei.gimbitskii@intel.com
State New
Headers show
Series "Fix issues reported by klocwork" ( rev: 2 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Aleksei Gimbitskii April 23, 2019, 12:04 p.m.
Static code analyzer warns that index value for scratch_pt may be equal
to -1. Index value type is intel_gvt_gtt_type_t, so it may be any number
at range -1 to 17. Check first if cur_pt_type and cur_pt_type+1 is valid
values.

v2:
 - Print some error messages if page table type is invalid. (Colin Xu)

This patch fixed the critial issue #422 reported by klocwork.

Signed-off-by: Aleksei Gimbitskii <aleksei.gimbitskii@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
Cc: Colin Xu <colin.xu@intel.com>
---
 drivers/gpu/drm/i915/gvt/gtt.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 7600416db908..061499a38f4d 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -942,7 +942,16 @@  static int ppgtt_invalidate_spt_by_shadow_entry(struct intel_vgpu *vgpu,
 
 	if (e->type != GTT_TYPE_PPGTT_ROOT_L3_ENTRY
 		&& e->type != GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
-		cur_pt_type = get_next_pt_type(e->type) + 1;
+		cur_pt_type = get_next_pt_type(e->type);
+
+		if (!gtt_type_is_pt(cur_pt_type) ||
+				!gtt_type_is_pt(cur_pt_type + 1)) {
+			WARN(1, "Invalid page table type\n");
+			return -EINVAL;
+		}
+
+		cur_pt_type += 1;
+
 		if (ops->get_pfn(e) ==
 			vgpu->gtt.scratch_pt[cur_pt_type].page_mfn)
 			return 0;

Comments

On 2019-04-23 20:04, Aleksei Gimbitskii wrote:
> Static code analyzer warns that index value for scratch_pt may be equal
> to -1. Index value type is intel_gvt_gtt_type_t, so it may be any number
> at range -1 to 17. Check first if cur_pt_type and cur_pt_type+1 is valid
> values.
>
> v2:
>   - Print some error messages if page table type is invalid. (Colin Xu)
>
> This patch fixed the critial issue #422 reported by klocwork.
>
> Signed-off-by: Aleksei Gimbitskii <aleksei.gimbitskii@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Cc: Colin Xu <colin.xu@intel.com>
> ---
>   drivers/gpu/drm/i915/gvt/gtt.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index 7600416db908..061499a38f4d 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -942,7 +942,16 @@ static int ppgtt_invalidate_spt_by_shadow_entry(struct intel_vgpu *vgpu,
>   
>   	if (e->type != GTT_TYPE_PPGTT_ROOT_L3_ENTRY
>   		&& e->type != GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
> -		cur_pt_type = get_next_pt_type(e->type) + 1;
> +		cur_pt_type = get_next_pt_type(e->type);
> +
> +		if (!gtt_type_is_pt(cur_pt_type) ||
> +				!gtt_type_is_pt(cur_pt_type + 1)) {
> +			WARN(1, "Invalid page table type\n");

Oh looks the comments in v2 also missed.
Would you add the actual invalid type it hits into the warning as well?

> +			return -EINVAL;
> +		}
> +
> +		cur_pt_type += 1;
> +
>   		if (ops->get_pfn(e) ==
>   			vgpu->gtt.scratch_pt[cur_pt_type].page_mfn)
>   			return 0;
Thanks for reminding and sorry for late response.
I will add cur_pt_type value to the WARN message.

-----Original Message-----
From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of Colin Xu

Sent: Thursday, April 25, 2019 8:43 AM
To: Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; intel-gvt-dev@lists.freedesktop.org
Cc: Wang, Zhi A <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
Subject: Re: [PATCH v3 4/6] drm/i915/gvt: Check if cur_pt_type is valid


On 2019-04-23 20:04, Aleksei Gimbitskii wrote:
> Static code analyzer warns that index value for scratch_pt may be 

> equal to -1. Index value type is intel_gvt_gtt_type_t, so it may be 

> any number at range -1 to 17. Check first if cur_pt_type and 

> cur_pt_type+1 is valid values.

>

> v2:

>   - Print some error messages if page table type is invalid. (Colin 

> Xu)

>

> This patch fixed the critial issue #422 reported by klocwork.

>

> Signed-off-by: Aleksei Gimbitskii <aleksei.gimbitskii@intel.com>

> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>

> Cc: Zhi Wang <zhi.a.wang@intel.com>

> Cc: Colin Xu <colin.xu@intel.com>

> ---

>   drivers/gpu/drm/i915/gvt/gtt.c | 11 ++++++++++-

>   1 file changed, 10 insertions(+), 1 deletion(-)

>

> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c 

> b/drivers/gpu/drm/i915/gvt/gtt.c index 7600416db908..061499a38f4d 

> 100644

> --- a/drivers/gpu/drm/i915/gvt/gtt.c

> +++ b/drivers/gpu/drm/i915/gvt/gtt.c

> @@ -942,7 +942,16 @@ static int 

> ppgtt_invalidate_spt_by_shadow_entry(struct intel_vgpu *vgpu,

>   

>   	if (e->type != GTT_TYPE_PPGTT_ROOT_L3_ENTRY

>   		&& e->type != GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {

> -		cur_pt_type = get_next_pt_type(e->type) + 1;

> +		cur_pt_type = get_next_pt_type(e->type);

> +

> +		if (!gtt_type_is_pt(cur_pt_type) ||

> +				!gtt_type_is_pt(cur_pt_type + 1)) {

> +			WARN(1, "Invalid page table type\n");


Oh looks the comments in v2 also missed.
Would you add the actual invalid type it hits into the warning as well?

> +			return -EINVAL;

> +		}

> +

> +		cur_pt_type += 1;

> +

>   		if (ops->get_pfn(e) ==

>   			vgpu->gtt.scratch_pt[cur_pt_type].page_mfn)

>   			return 0;


--
Best Regards,
Colin Xu

_______________________________________________
intel-gvt-dev mailing list
intel-gvt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
On 2019-04-29 20:23, Gimbitskii, Aleksei wrote:
> Thanks for reminding and sorry for late response.
> I will add cur_pt_type value to the WARN message.

As mentioned in 5/6, it's also OK to keep current style and improve all msg in future.

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of Colin Xu
> Sent: Thursday, April 25, 2019 8:43 AM
> To: Gimbitskii, Aleksei <aleksei.gimbitskii@intel.com>; intel-gvt-dev@lists.freedesktop.org
> Cc: Wang, Zhi A <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> Subject: Re: [PATCH v3 4/6] drm/i915/gvt: Check if cur_pt_type is valid
>
>
> On 2019-04-23 20:04, Aleksei Gimbitskii wrote:
>> Static code analyzer warns that index value for scratch_pt may be
>> equal to -1. Index value type is intel_gvt_gtt_type_t, so it may be
>> any number at range -1 to 17. Check first if cur_pt_type and
>> cur_pt_type+1 is valid values.
>>
>> v2:
>>    - Print some error messages if page table type is invalid. (Colin
>> Xu)
>>
>> This patch fixed the critial issue #422 reported by klocwork.
>>
>> Signed-off-by: Aleksei Gimbitskii <aleksei.gimbitskii@intel.com>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: Zhi Wang <zhi.a.wang@intel.com>
>> Cc: Colin Xu <colin.xu@intel.com>
>> ---
>>    drivers/gpu/drm/i915/gvt/gtt.c | 11 ++++++++++-
>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c
>> b/drivers/gpu/drm/i915/gvt/gtt.c index 7600416db908..061499a38f4d
>> 100644
>> --- a/drivers/gpu/drm/i915/gvt/gtt.c
>> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
>> @@ -942,7 +942,16 @@ static int
>> ppgtt_invalidate_spt_by_shadow_entry(struct intel_vgpu *vgpu,
>>    
>>    	if (e->type != GTT_TYPE_PPGTT_ROOT_L3_ENTRY
>>    		&& e->type != GTT_TYPE_PPGTT_ROOT_L4_ENTRY) {
>> -		cur_pt_type = get_next_pt_type(e->type) + 1;
>> +		cur_pt_type = get_next_pt_type(e->type);
>> +
>> +		if (!gtt_type_is_pt(cur_pt_type) ||
>> +				!gtt_type_is_pt(cur_pt_type + 1)) {
>> +			WARN(1, "Invalid page table type\n");
> Oh looks the comments in v2 also missed.
> Would you add the actual invalid type it hits into the warning as well?
>
>> +			return -EINVAL;
>> +		}
>> +
>> +		cur_pt_type += 1;
>> +
>>    		if (ops->get_pfn(e) ==
>>    			vgpu->gtt.scratch_pt[cur_pt_type].page_mfn)
>>    			return 0;
> --
> Best Regards,
> Colin Xu
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev