[PATCH 0/6] x86: address remaining rule 18.2 violations and mark clean

[PATCH 0/6] x86: address remaining rule 18.2 violations and mark clean

[Jan Beulich]

Arm64 is already clean. And all four analysis jobs have already been clean
for rule 18.1.

1: PV: adjust APPEND_CALL() to comply to Misra rule 18.2
2: alternatives: adjust _apply_alternatives() to comply to Misra rule 18.2
3: EFI: adjust efi_multiboot2_prelude() to comply to Misra rule 18.2
4: cpufreq: annotate Eclair false-positives for rule 18.2
5: Viridian: annotate Eclair false-positives for rule 18.2
6: automation/Eclair: tag rules 18.1 and 18.2 as clean

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/2832933871

Jan

[PATCH 1/6] x86/PV: adjust APPEND_CALL() to comply to Misra rule 18.2

[Jan Beulich]

While casting to pointer types may be more natural there, the subtraction
then ends up violating "Subtraction between pointers shall only be applied
to pointers that address elements of the same array". Use long arithmetic
instead.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Depends on "Eclair: relax long <-> function-pointer conversion deviation"
to not introduce other violations in turn.

--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -92,7 +92,8 @@ static io_emul_stub_t *io_emul_stub_setu
 #define APPEND_BUFF(b) ({ memcpy(p, b, sizeof(b)); p += sizeof(b); })
 #define APPEND_CALL(f)                                                  \
     ({                                                                  \
-        long disp = (void *)(f) - (stub_va + (p - ctxt->io_emul_stub) + 5); \
+        long disp = (long)(f) -                                         \
+                    ((long)stub_va + (p - ctxt->io_emul_stub) + 5);     \
         BUG_ON((int32_t)disp != disp);                                  \
         *p++ = 0xe8;                                                    \
         *(int32_t *)p = disp; p += 4;                                   \

[PATCH 2/6] x86/alternatives: adjust _apply_alternatives() to comply to Misra rule 18.2

[Jan Beulich]

While avoiding casts would be preferable here, the subtraction ends up
violating "Subtraction between pointers shall only be applied to pointers
that address elements of the same array". Use long arithmetic instead.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -342,13 +342,13 @@ static int init_or_livepatch _apply_alte
 
         /* 0xe8/0xe9 are relative branches; fix the offset. */
         if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 )
-            *(int32_t *)(buf + 1) += repl - orig;
+            *(int32_t *)(buf + 1) += (long)repl - (long)orig;
         else if ( IS_ENABLED(CONFIG_RETURN_THUNK) &&
                   a->repl_len > 5 && buf[a->repl_len - 5] == 0xe9 &&
                   ((long)repl + a->repl_len +
                    *(int32_t *)(buf + a->repl_len - 4) ==
                    (long)__x86_return_thunk) )
-            *(int32_t *)(buf + a->repl_len - 4) += repl - orig;
+            *(int32_t *)(buf + a->repl_len - 4) += (long)repl - (long)orig;
 
         a->priv = 1;
 

[PATCH 3/6] x86/EFI: adjust efi_multiboot2_prelude() to comply to Misra rule 18.2

[Jan Beulich]

While casting to pointer types may be more natural there, the subtraction
then ends up violating "Subtraction between pointers shall only be applied
to pointers that address elements of the same array". Use unsigned long
arithmetic instead.

No functional change intended.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/efi/mbi2.c
+++ b/xen/arch/x86/efi/mbi2.c
@@ -13,7 +13,7 @@ efi_multiboot2_prelude(uint32_t magic, c
     EFI_HANDLE ImageHandle = NULL;
     EFI_SYSTEM_TABLE *SystemTable = NULL;
     const char *cmdline = NULL;
-    const void *const mbi_raw = (const void *)mbi;
+    unsigned long mbi_raw = (unsigned long)mbi;
     bool have_bs = false;
 
     if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC )
@@ -22,10 +22,10 @@ efi_multiboot2_prelude(uint32_t magic, c
     /* Skip Multiboot2 information fixed part. */
     tag = _p(ROUNDUP((unsigned long)(mbi + 1), MULTIBOOT2_TAG_ALIGN));
 
-    for ( ; (const void *)(tag + 1) - mbi_raw <= mbi->total_size &&
+    for ( ; (unsigned long)(tag + 1) - mbi_raw <= mbi->total_size &&
             tag->type != MULTIBOOT2_TAG_TYPE_END &&
             tag->size >= sizeof(*tag) &&
-            (const void *)tag + tag->size - mbi_raw <= mbi->total_size;
+            (unsigned long)tag + tag->size - mbi_raw <= mbi->total_size;
           tag = _p(ROUNDUP((unsigned long)tag + tag->size,
                    MULTIBOOT2_TAG_ALIGN)) )
     {

[PATCH 4/6] x86/cpufreq: annotate Eclair false-positives for rule 18.2

[Jan Beulich]

"Subtraction between pointers shall only be applied to pointers that
address elements of the same array" is not violated here. Elsewhere, on
simple "e - s" Eclair manages to notice this; apparently "(end ?: e)" is
too complex ("end" being derived from "s" a few lines earlier).

Suggested-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
As per Nicola Eclair 16.0.0 has this fixed.

--- a/docs/misra/false-positive-eclair.json
+++ b/docs/misra/false-positive-eclair.json
@@ -3,6 +3,13 @@
     "content": [
         {
             "id": "SAF-0-false-positive-eclair",
+            "violation-id": "MC3A2.R18.2",
+            "tool-version": "3.14.0",
+            "name": "Rule 18.2: pointer subtraction",
+            "text": "Cmdline parsing start/end pointers point into the same array"
+        },
+        {
+            "id": "SAF-1-false-positive-eclair",
             "violation-id": "",
             "tool-version": "",
             "name": "Sentinel",
--- a/xen/arch/x86/acpi/cpufreq/amd-cppc.c
+++ b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
@@ -67,6 +67,7 @@ int __init amd_cppc_cmdline_parse(const
         {
             printk(XENLOG_WARNING
                    "cpufreq/amd-cppc: option '%.*s' not recognized\n",
+                   /* SAF-0-false-positive-eclair s, e, and end point into the cmdline array */
                    (int)((end ?: e) - s), s);
 
             return -EINVAL;
--- a/xen/arch/x86/acpi/cpufreq/hwp.c
+++ b/xen/arch/x86/acpi/cpufreq/hwp.c
@@ -85,6 +85,7 @@ int __init hwp_cmdline_parse(const char
         if ( !hwp_handle_option(s, end) )
         {
             printk(XENLOG_WARNING "cpufreq/hwp: option '%.*s' not recognized\n",
+                   /* SAF-0-false-positive-eclair s, e, and end point into the cmdline array */
                    (int)((end ?: e) - s), s);
 
             return -EINVAL;

[PATCH 5/6] x86/Viridian: annotate Eclair false-positives for rule 18.2

[Jan Beulich]

"Subtraction between pointers shall only be applied to pointers that
address elements of the same array" is not violated here: start_stimer()
is only ever passed a sane argument, and stimer_expire() is either called
from start_stimer() (using its parameter as argument) or as a callback,
where a sane callback argument is also guaranteed to be set up.

Suggested-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
As per Nicola Eclair 16.0.0 has this fixed.

--- a/docs/misra/false-positive-eclair.json
+++ b/docs/misra/false-positive-eclair.json
@@ -10,6 +10,13 @@
         },
         {
             "id": "SAF-1-false-positive-eclair",
+            "violation-id": "MC3A2.R18.2",
+            "tool-version": "3.14.0",
+            "name": "Rule 18.2: pointer subtraction",
+            "text": "Viridian stimer index calculations use sane pointers"
+        },
+        {
+            "id": "SAF-2-false-positive-eclair",
             "violation-id": "",
             "tool-version": "",
             "name": "Sentinel",
--- a/xen/arch/x86/hvm/viridian/time.c
+++ b/xen/arch/x86/hvm/viridian/time.c
@@ -136,6 +136,7 @@ static void cf_check stimer_expire(void
     struct viridian_stimer *vs = data;
     struct vcpu *v = vs->v;
     struct viridian_vcpu *vv = v->arch.hvm.viridian;
+    /* SAF-1-false-positive-eclair vs is always sane */
     unsigned int stimerx = vs - &vv->stimer[0];
 
     set_bit(stimerx, &vv->stimer_pending);
@@ -146,6 +147,7 @@ static void start_stimer(struct viridian
 {
     const struct vcpu *v = vs->v;
     struct viridian_vcpu *vv = v->arch.hvm.viridian;
+    /* SAF-1-false-positive-eclair vs is always sane */
     unsigned int stimerx = vs - &vv->stimer[0];
     int64_t now = time_ref_count(v->domain);
     int64_t expiration;

[PATCH 6/6] automation/Eclair: tag rules 18.1 and 18.2 as clean

[Jan Beulich]

Remaining (x86) 18.2 violations were addressed. 18.1 was already clean
everywhere.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/automation/eclair_analysis/ECLAIR/tagging.ecl
+++ b/automation/eclair_analysis/ECLAIR/tagging.ecl
@@ -80,6 +80,8 @@ MC3A2.R17.3||
 MC3A2.R17.4||
 MC3A2.R17.5||
 MC3A2.R17.6||
+MC3A2.R18.1||
+MC3A2.R18.2||
 MC3A2.R18.6||
 MC3A2.R18.8||
 MC3A2.R19.1||

Re: [PATCH 5/6] x86/Viridian: annotate Eclair false-positives for rule 18.2

[Nicola Vetrini]

On 2026-09-09 15:01, Jan Beulich wrote:
> "Subtraction between pointers shall only be applied to pointers that
> address elements of the same array" is not violated here: 
> start_stimer()
> is only ever passed a sane argument, and stimer_expire() is either 
> called
> from start_stimer() (using its parameter as argument) or as a callback,
> where a sane callback argument is also guaranteed to be set up.
> 
> Suggested-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> As per Nicola Eclair 16.0.0 has this fixed.

Perhaps I didn't express clearly that the remark was about the violation 
annotated in patch 4/6. Here instead, a suitable reproducer is not yet 
available. Nevertheless, if the claim is true the approach to address 
the violation is still ok.

> 
> --- a/docs/misra/false-positive-eclair.json
> +++ b/docs/misra/false-positive-eclair.json
> @@ -10,6 +10,13 @@
>          },
>          {
>              "id": "SAF-1-false-positive-eclair",
> +            "violation-id": "MC3A2.R18.2",
> +            "tool-version": "3.14.0",
> +            "name": "Rule 18.2: pointer subtraction",
> +            "text": "Viridian stimer index calculations use sane 
> pointers"
> +        },
> +        {
> +            "id": "SAF-2-false-positive-eclair",
>              "violation-id": "",
>              "tool-version": "",
>              "name": "Sentinel",
> --- a/xen/arch/x86/hvm/viridian/time.c
> +++ b/xen/arch/x86/hvm/viridian/time.c
> @@ -136,6 +136,7 @@ static void cf_check stimer_expire(void
>      struct viridian_stimer *vs = data;
>      struct vcpu *v = vs->v;
>      struct viridian_vcpu *vv = v->arch.hvm.viridian;
> +    /* SAF-1-false-positive-eclair vs is always sane */
>      unsigned int stimerx = vs - &vv->stimer[0];
> 
>      set_bit(stimerx, &vv->stimer_pending);
> @@ -146,6 +147,7 @@ static void start_stimer(struct viridian
>  {
>      const struct vcpu *v = vs->v;
>      struct viridian_vcpu *vv = v->arch.hvm.viridian;
> +    /* SAF-1-false-positive-eclair vs is always sane */
>      unsigned int stimerx = vs - &vv->stimer[0];
>      int64_t now = time_ref_count(v->domain);
>      int64_t expiration;

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 3/6] x86/EFI: adjust efi_multiboot2_prelude() to comply to Misra rule 18.2

[Marek Marczykowski]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

On Wed, Sep 09, 2026 at 03:00:21PM +0200, Jan Beulich wrote:
> While casting to pointer types may be more natural there, the subtraction
> then ends up violating "Subtraction between pointers shall only be applied
> to pointers that address elements of the same array". Use unsigned long
> arithmetic instead.
> 
> No functional change intended.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I was going to propose pre-calculating mbi_raw + mbi->total_size and
checking against that, but then checking for overflow would need to be
explicit. So, your version indeed looks better.

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]