In the file common/kernel.c macro ARRAY_SIZE is called with argument
current->domain->handle.
Once expanded, this ARRAY_SIZE's argument is used in sizeof operations
and thus 'current', being a macro that expands to a function
call with potential side effects, generates a violation.

To address this violation the value of current->domain is therefore
stored in a variable called 'd' before passing it to macro ARRAY_SIZE.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 xen/common/kernel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index b44b2439ca..76b4f27aef 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -660,14 +660,15 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 
     case XENVER_guest_handle:
     {
+        struct domain *d = current->domain;
         xen_domain_handle_t hdl;
 
         if ( deny )
             memset(&hdl, 0, ARRAY_SIZE(hdl));
 
-        BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl));
+        BUILD_BUG_ON(ARRAY_SIZE(d->handle) != ARRAY_SIZE(hdl));
 
-        if ( copy_to_guest(arg, deny ? hdl : current->domain->handle,
+        if ( copy_to_guest(arg, deny ? hdl : d->handle,
                            ARRAY_SIZE(hdl) ) )
             return -EFAULT;
         return 0;
-- 
2.34.1

On 25.06.2024 12:14, Alessandro Zucchelli wrote:
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -660,14 +660,15 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>      case XENVER_guest_handle:
>      {
> +        struct domain *d = current->domain;

Can a (new) variable thus initialized please be consistently named currd?

>          xen_domain_handle_t hdl;
>  
>          if ( deny )
>              memset(&hdl, 0, ARRAY_SIZE(hdl));
>  
> -        BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl));
> +        BUILD_BUG_ON(ARRAY_SIZE(d->handle) != ARRAY_SIZE(hdl));

Wasn't there the intention to exclude BUILD_BUG_ON() for specifically this
(and any other similar) rule?

Jan

> -        if ( copy_to_guest(arg, deny ? hdl : current->domain->handle,
> +        if ( copy_to_guest(arg, deny ? hdl : d->handle,
>                             ARRAY_SIZE(hdl) ) )
>              return -EFAULT;
>          return 0;

On Tue, 25 Jun 2024, Jan Beulich wrote:
> On 25.06.2024 12:14, Alessandro Zucchelli wrote:
> > --- a/xen/common/kernel.c
> > +++ b/xen/common/kernel.c
> > @@ -660,14 +660,15 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> >  
> >      case XENVER_guest_handle:
> >      {
> > +        struct domain *d = current->domain;
> 
> Can a (new) variable thus initialized please be consistently named currd?
> 
> >          xen_domain_handle_t hdl;
> >  
> >          if ( deny )
> >              memset(&hdl, 0, ARRAY_SIZE(hdl));
> >  
> > -        BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl));
> > +        BUILD_BUG_ON(ARRAY_SIZE(d->handle) != ARRAY_SIZE(hdl));
> 
> Wasn't there the intention to exclude BUILD_BUG_ON() for specifically this
> (and any other similar) rule?

+1

I think if we could do that it would be ideal because those are the
difficult cases are only meant are build checks so there is no point in
applying to MISRA to them.


> > -        if ( copy_to_guest(arg, deny ? hdl : current->domain->handle,
> > +        if ( copy_to_guest(arg, deny ? hdl : d->handle,
> >                             ARRAY_SIZE(hdl) ) )
> >              return -EFAULT;
> >          return 0;
>

On 2024-06-26 03:13, Stefano Stabellini wrote:

Hi,
> On Tue, 25 Jun 2024, Jan Beulich wrote:
>> On 25.06.2024 12:14, Alessandro Zucchelli wrote:
>> > --- a/xen/common/kernel.c
>> > +++ b/xen/common/kernel.c
>> > @@ -660,14 +660,15 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>> >
>> >      case XENVER_guest_handle:
>> >      {
>> > +        struct domain *d = current->domain;
>> 
>> Can a (new) variable thus initialized please be consistently named 
>> currd?
>> 
>> >          xen_domain_handle_t hdl;
>> >
>> >          if ( deny )
>> >              memset(&hdl, 0, ARRAY_SIZE(hdl));
>> >
>> > -        BUILD_BUG_ON(ARRAY_SIZE(current->domain->handle) != ARRAY_SIZE(hdl));
>> > +        BUILD_BUG_ON(ARRAY_SIZE(d->handle) != ARRAY_SIZE(hdl));
>> 
>> Wasn't there the intention to exclude BUILD_BUG_ON() for specifically 
>> this
>> (and any other similar) rule?
> 
> +1

Yes, this macro will be deviated, you may ignore this patch.

> 
> I think if we could do that it would be ideal because those are the
> difficult cases are only meant are build checks so there is no point in
> applying to MISRA to them.
> 
> 
>> > -        if ( copy_to_guest(arg, deny ? hdl : current->domain->handle,
>> > +        if ( copy_to_guest(arg, deny ? hdl : d->handle,
>> >                             ARRAY_SIZE(hdl) ) )
>> >              return -EFAULT;
>> >          return 0;
>> 

-- 
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)