* [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
@ 2021-04-13 19:54 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2021-04-13 19:54 UTC (permalink / raw)
To: David Woodhouse, Lu Baolu, Joerg Roedel, Will Deacon
Cc: iommu, linux-kernel, Gustavo A. R. Silva, linux-hardening
Replace call to memcpy() with just a couple of simple assignments in
order to fix the following out-of-bounds warning:
drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type 'long long unsigned int' at offset 16 [-Warray-bounds]
The problem is that the original code is trying to copy data into a
couple of struct members adjacent to each other in a single call to
memcpy(). This causes a legitimate compiler warning because memcpy()
overruns the length of &desc.qw2.
This helps with the ongoing efforts to globally enable -Warray-bounds
and get us closer to being able to tighten the FORTIFY_SOURCE routines
on memcpy().
Link: https://github.com/KSPP/linux/issues/109
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/iommu/intel/svm.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 5165cea90421..65909f504c50 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev,
desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page);
desc.qw2 = 0;
desc.qw3 = 0;
- if (private_present)
- memcpy(&desc.qw2, prm->private_data,
- sizeof(prm->private_data));
+ if (private_present) {
+ desc.qw2 = prm->private_data[0];
+ desc.qw3 = prm->private_data[1];
+ }
qi_submit_sync(iommu, &desc, 1, 0);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
@ 2021-04-13 19:54 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2021-04-13 19:54 UTC (permalink / raw)
To: David Woodhouse, Lu Baolu, Joerg Roedel, Will Deacon
Cc: iommu, linux-hardening, linux-kernel, Gustavo A. R. Silva
Replace call to memcpy() with just a couple of simple assignments in
order to fix the following out-of-bounds warning:
drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type 'long long unsigned int' at offset 16 [-Warray-bounds]
The problem is that the original code is trying to copy data into a
couple of struct members adjacent to each other in a single call to
memcpy(). This causes a legitimate compiler warning because memcpy()
overruns the length of &desc.qw2.
This helps with the ongoing efforts to globally enable -Warray-bounds
and get us closer to being able to tighten the FORTIFY_SOURCE routines
on memcpy().
Link: https://github.com/KSPP/linux/issues/109
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/iommu/intel/svm.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 5165cea90421..65909f504c50 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev,
desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page);
desc.qw2 = 0;
desc.qw3 = 0;
- if (private_present)
- memcpy(&desc.qw2, prm->private_data,
- sizeof(prm->private_data));
+ if (private_present) {
+ desc.qw2 = prm->private_data[0];
+ desc.qw3 = prm->private_data[1];
+ }
qi_submit_sync(iommu, &desc, 1, 0);
}
--
2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
2021-04-13 19:54 ` Gustavo A. R. Silva
@ 2021-04-14 5:24 ` Lu Baolu
-1 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2021-04-14 5:24 UTC (permalink / raw)
To: Gustavo A. R. Silva, David Woodhouse, Joerg Roedel, Will Deacon
Cc: baolu.lu, iommu, linux-kernel, linux-hardening
Hi Gustavo,
On 4/14/21 3:54 AM, Gustavo A. R. Silva wrote:
> Replace call to memcpy() with just a couple of simple assignments in
> order to fix the following out-of-bounds warning:
>
> drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type 'long long unsigned int' at offset 16 [-Warray-bounds]
>
> The problem is that the original code is trying to copy data into a
> couple of struct members adjacent to each other in a single call to
> memcpy(). This causes a legitimate compiler warning because memcpy()
> overruns the length of &desc.qw2.
>
> This helps with the ongoing efforts to globally enable -Warray-bounds
> and get us closer to being able to tighten the FORTIFY_SOURCE routines
> on memcpy().
>
> Link: https://github.com/KSPP/linux/issues/109
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> drivers/iommu/intel/svm.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> index 5165cea90421..65909f504c50 100644
> --- a/drivers/iommu/intel/svm.c
> +++ b/drivers/iommu/intel/svm.c
> @@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev,
> desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page);
> desc.qw2 = 0;
> desc.qw3 = 0;
> - if (private_present)
> - memcpy(&desc.qw2, prm->private_data,
> - sizeof(prm->private_data));
The same memcpy() is used in multiple places in this file. Did they
compile the same warnings? Or there are multiple patches to fix them
one by one?
Best regards,
baolu
> + if (private_present) {
> + desc.qw2 = prm->private_data[0];
> + desc.qw3 = prm->private_data[1];
> + }
>
> qi_submit_sync(iommu, &desc, 1, 0);
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
@ 2021-04-14 5:24 ` Lu Baolu
0 siblings, 0 replies; 6+ messages in thread
From: Lu Baolu @ 2021-04-14 5:24 UTC (permalink / raw)
To: Gustavo A. R. Silva, David Woodhouse, Joerg Roedel, Will Deacon
Cc: iommu, linux-hardening, linux-kernel
Hi Gustavo,
On 4/14/21 3:54 AM, Gustavo A. R. Silva wrote:
> Replace call to memcpy() with just a couple of simple assignments in
> order to fix the following out-of-bounds warning:
>
> drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type 'long long unsigned int' at offset 16 [-Warray-bounds]
>
> The problem is that the original code is trying to copy data into a
> couple of struct members adjacent to each other in a single call to
> memcpy(). This causes a legitimate compiler warning because memcpy()
> overruns the length of &desc.qw2.
>
> This helps with the ongoing efforts to globally enable -Warray-bounds
> and get us closer to being able to tighten the FORTIFY_SOURCE routines
> on memcpy().
>
> Link: https://github.com/KSPP/linux/issues/109
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> drivers/iommu/intel/svm.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> index 5165cea90421..65909f504c50 100644
> --- a/drivers/iommu/intel/svm.c
> +++ b/drivers/iommu/intel/svm.c
> @@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev,
> desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page);
> desc.qw2 = 0;
> desc.qw3 = 0;
> - if (private_present)
> - memcpy(&desc.qw2, prm->private_data,
> - sizeof(prm->private_data));
The same memcpy() is used in multiple places in this file. Did they
compile the same warnings? Or there are multiple patches to fix them
one by one?
Best regards,
baolu
> + if (private_present) {
> + desc.qw2 = prm->private_data[0];
> + desc.qw3 = prm->private_data[1];
> + }
>
> qi_submit_sync(iommu, &desc, 1, 0);
> }
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
2021-04-14 5:24 ` Lu Baolu
@ 2021-04-14 20:04 ` Gustavo A. R. Silva
-1 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2021-04-14 20:04 UTC (permalink / raw)
To: Lu Baolu, Gustavo A. R. Silva, David Woodhouse, Joerg Roedel,
Will Deacon
Cc: iommu, linux-kernel, linux-hardening
Hi Balou,
On 4/14/21 00:24, Lu Baolu wrote:
> Hi Gustavo,
>
> On 4/14/21 3:54 AM, Gustavo A. R. Silva wrote:
>> Replace call to memcpy() with just a couple of simple assignments in
>> order to fix the following out-of-bounds warning:
>>
>> drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type
>> 'long long unsigned int' at offset 16 [-Warray-bounds]
>>
>> The problem is that the original code is trying to copy data into a
>> couple of struct members adjacent to each other in a single call to
>> memcpy(). This causes a legitimate compiler warning because memcpy()
>> overruns the length of &desc.qw2.
>>
>> This helps with the ongoing efforts to globally enable -Warray-bounds
>> and get us closer to being able to tighten the FORTIFY_SOURCE routines
>> on memcpy().
>>
>> Link: https://github.com/KSPP/linux/issues/109
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>> drivers/iommu/intel/svm.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
>> index 5165cea90421..65909f504c50 100644
>> --- a/drivers/iommu/intel/svm.c
>> +++ b/drivers/iommu/intel/svm.c
>> @@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev,
>> desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page);
>> desc.qw2 = 0;
>> desc.qw3 = 0;
>> - if (private_present)
>> - memcpy(&desc.qw2, prm->private_data,
>> - sizeof(prm->private_data));
>
> The same memcpy() is used in multiple places in this file. Did they
> compile the same warnings? Or there are multiple patches to fix them
> one by one?
I just see one more instance of this same case:
1023 if (req->priv_data_present)
1024 memcpy(&resp.qw2, req->priv_data,
1025 sizeof(req->priv_data));
I missed it and I'll address it in v2. Do you see another one?
Thanks for the feedback!
--
Gustavo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response()
@ 2021-04-14 20:04 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2021-04-14 20:04 UTC (permalink / raw)
To: Lu Baolu, Gustavo A. R. Silva, David Woodhouse, Joerg Roedel,
Will Deacon
Cc: iommu, linux-kernel, linux-hardening
Hi Balou,
On 4/14/21 00:24, Lu Baolu wrote:
> Hi Gustavo,
>
> On 4/14/21 3:54 AM, Gustavo A. R. Silva wrote:
>> Replace call to memcpy() with just a couple of simple assignments in
>> order to fix the following out-of-bounds warning:
>>
>> drivers/iommu/intel/svm.c:1198:4: warning: 'memcpy' offset [25, 32] from the object at 'desc' is out of the bounds of referenced subobject 'qw2' with type
>> 'long long unsigned int' at offset 16 [-Warray-bounds]
>>
>> The problem is that the original code is trying to copy data into a
>> couple of struct members adjacent to each other in a single call to
>> memcpy(). This causes a legitimate compiler warning because memcpy()
>> overruns the length of &desc.qw2.
>>
>> This helps with the ongoing efforts to globally enable -Warray-bounds
>> and get us closer to being able to tighten the FORTIFY_SOURCE routines
>> on memcpy().
>>
>> Link: https://github.com/KSPP/linux/issues/109
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>> drivers/iommu/intel/svm.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
>> index 5165cea90421..65909f504c50 100644
>> --- a/drivers/iommu/intel/svm.c
>> +++ b/drivers/iommu/intel/svm.c
>> @@ -1194,9 +1194,10 @@ int intel_svm_page_response(struct device *dev,
>> desc.qw1 = QI_PGRP_IDX(prm->grpid) | QI_PGRP_LPIG(last_page);
>> desc.qw2 = 0;
>> desc.qw3 = 0;
>> - if (private_present)
>> - memcpy(&desc.qw2, prm->private_data,
>> - sizeof(prm->private_data));
>
> The same memcpy() is used in multiple places in this file. Did they
> compile the same warnings? Or there are multiple patches to fix them
> one by one?
I just see one more instance of this same case:
1023 if (req->priv_data_present)
1024 memcpy(&resp.qw2, req->priv_data,
1025 sizeof(req->priv_data));
I missed it and I'll address it in v2. Do you see another one?
Thanks for the feedback!
--
Gustavo
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-14 20:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 19:54 [PATCH][next] iommu/vt-d: Fix out-bounds-warning in intel_svm_page_response() Gustavo A. R. Silva
2021-04-13 19:54 ` Gustavo A. R. Silva
2021-04-14 5:24 ` Lu Baolu
2021-04-14 5:24 ` Lu Baolu
2021-04-14 20:04 ` Gustavo A. R. Silva
2021-04-14 20:04 ` Gustavo A. R. Silva
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.