diff mbox series

[v2,4/6] xen: add SAF deviation for safe cast removal

Message ID dff9e788e04aa04970cfbb70d09f4d1baf725506.1702982442.git.maria.celeste.cesario@bugseng.com (mailing list archive)
State New
Headers show
Series xen: address violations of MISRA C:2012 Rule 11.8 | expand

Commit Message

Simone Ballarin Dec. 19, 2023, 11:05 a.m. UTC 
From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>

The xen sources contain violations of MISRA C:2012 Rule 11.8 whose
headline states:
"A conversion shall not remove any const, volatile or _Atomic qualification
from the type pointed to by a pointer".

Add SAF-3-safe deviation: removal of const qualifier to comply with function signature.

Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
---
Changes in v2:
- reword SAF-3-safe text;
- merge comments on __hvm_copy;
- add SAF-3-safe comment in x86/hvm.c:3433;
- add SAF-3-safe comment on arm/guestcopy.c raw_copy_to_guest and
  raw_copy_to_guest_flush_dcache.
---
 docs/misra/safe.json     | 8 ++++++++
 xen/arch/arm/guestcopy.c | 2 ++
 xen/arch/x86/hvm/hvm.c   | 6 ++++--
 3 files changed, 14 insertions(+), 2 deletions(-)

Comments

Jan Beulich Dec. 19, 2023, 11:28 a.m. UTC | #1 
On 19.12.2023 12:05, Simone Ballarin wrote:
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -109,6 +109,7 @@ static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
>  
>  unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
>  {
> +    /* SAF-3-safe COPY_to_guest doesn't modify from */
>      return copy_guest((void *)from, (vaddr_t)to, len,
>                        GVA_INFO(current), COPY_to_guest | COPY_linear);
>  }
> @@ -116,6 +117,7 @@ unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
>  unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
>                                               unsigned int len)
>  {
> +    /* SAF-3-safe COPY_to_guest doesn't modify from */
>      return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
>                        COPY_to_guest | COPY_flush_dcache | COPY_linear);
>  }

Unlike below for x86, here in both cases the comment cover more than
just the one function argument they are intended to cover. I think we
want to limit the scope of such comments as much as possible (and
hence have as little as possible on the immediately following line).

Jan

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3413,7 +3413,8 @@ static enum hvm_translation_result __hvm_copy(
>  enum hvm_translation_result hvm_copy_to_guest_phys(
>      paddr_t paddr, const void *buf, unsigned int size, struct vcpu *v)
>  {
> -    return __hvm_copy((void *)buf /* HVMCOPY_to_guest doesn't modify */,
> +    /* SAF-3-safe HVMCOPY_to_guest doesn't modify buf */
> +    return __hvm_copy((void *)buf,
>                        paddr, size, v,
>                        HVMCOPY_to_guest | HVMCOPY_phys, 0, NULL);
>  }
> @@ -3429,7 +3430,8 @@ enum hvm_translation_result hvm_copy_to_guest_linear(
>      unsigned long addr, const void *buf, unsigned int size, uint32_t pfec,
>      pagefault_info_t *pfinfo)
>  {
> -    return __hvm_copy((void *)buf /* HVMCOPY_to_guest doesn't modify */,
> +    /* SAF-3-safe HVMCOPY_to_guest doesn't modify buf */
> +    return __hvm_copy((void *)buf,
>                        addr, size, current, HVMCOPY_to_guest | HVMCOPY_linear,
>                        PFEC_page_present | PFEC_write_access | pfec, pfinfo);
>  }
Nicola Vetrini Dec. 19, 2023, 3:03 p.m. UTC | #2 
On 2023-12-19 12:28, Jan Beulich wrote:
> On 19.12.2023 12:05, Simone Ballarin wrote:
>> --- a/xen/arch/arm/guestcopy.c
>> +++ b/xen/arch/arm/guestcopy.c
>> @@ -109,6 +109,7 @@ static unsigned long copy_guest(void *buf, 
>> uint64_t addr, unsigned int len,
>> 
>>  unsigned long raw_copy_to_guest(void *to, const void *from, unsigned 
>> int len)
>>  {
>> +    /* SAF-3-safe COPY_to_guest doesn't modify from */
>>      return copy_guest((void *)from, (vaddr_t)to, len,
>>                        GVA_INFO(current), COPY_to_guest | 
>> COPY_linear);
>>  }
>> @@ -116,6 +117,7 @@ unsigned long raw_copy_to_guest(void *to, const 
>> void *from, unsigned int len)
>>  unsigned long raw_copy_to_guest_flush_dcache(void *to, const void 
>> *from,
>>                                               unsigned int len)
>>  {
>> +    /* SAF-3-safe COPY_to_guest doesn't modify from */
>>      return copy_guest((void *)from, (vaddr_t)to, len, 
>> GVA_INFO(current),
>>                        COPY_to_guest | COPY_flush_dcache | 
>> COPY_linear);
>>  }
> 
> Unlike below for x86, here in both cases the comment cover more than
> just the one function argument they are intended to cover. I think we
> want to limit the scope of such comments as much as possible (and
> hence have as little as possible on the immediately following line).
> 
> Jan

Ok, noted for v3.
diff mbox series

Patch

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 952324f85c..96b964293a 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -28,6 +28,14 @@ 
         },
         {
             "id": "SAF-3-safe",
+            "analyser": {
+                "eclair": "MC3R1.R11.8"
+            },
+            "name": "MC3R1.R11.8: removal of const qualifier to comply with function signature",
+            "text": "A single function could either read or write through a passed in pointer, depending on how it is called. It is deemed safe to cast away a const qualifier when passing a pointer to such a function, when the other parameters guarantee read-only operation."
+        },
+        {
+            "id": "SAF-4-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 6716b03561..cf80ac46b1 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -109,6 +109,7 @@  static unsigned long copy_guest(void *buf, uint64_t addr, unsigned int len,
 
 unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
 {
+    /* SAF-3-safe COPY_to_guest doesn't modify from */
     return copy_guest((void *)from, (vaddr_t)to, len,
                       GVA_INFO(current), COPY_to_guest | COPY_linear);
 }
@@ -116,6 +117,7 @@  unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
 unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
                                              unsigned int len)
 {
+    /* SAF-3-safe COPY_to_guest doesn't modify from */
     return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
                       COPY_to_guest | COPY_flush_dcache | COPY_linear);
 }
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 523e0df57c..324893fbcc 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3413,7 +3413,8 @@  static enum hvm_translation_result __hvm_copy(
 enum hvm_translation_result hvm_copy_to_guest_phys(
     paddr_t paddr, const void *buf, unsigned int size, struct vcpu *v)
 {
-    return __hvm_copy((void *)buf /* HVMCOPY_to_guest doesn't modify */,
+    /* SAF-3-safe HVMCOPY_to_guest doesn't modify buf */
+    return __hvm_copy((void *)buf,
                       paddr, size, v,
                       HVMCOPY_to_guest | HVMCOPY_phys, 0, NULL);
 }
@@ -3429,7 +3430,8 @@  enum hvm_translation_result hvm_copy_to_guest_linear(
     unsigned long addr, const void *buf, unsigned int size, uint32_t pfec,
     pagefault_info_t *pfinfo)
 {
-    return __hvm_copy((void *)buf /* HVMCOPY_to_guest doesn't modify */,
+    /* SAF-3-safe HVMCOPY_to_guest doesn't modify buf */
+    return __hvm_copy((void *)buf,
                       addr, size, current, HVMCOPY_to_guest | HVMCOPY_linear,
                       PFEC_page_present | PFEC_write_access | pfec, pfinfo);
 }