1. Patchew
  2. Xen
  3. View series

[PATCH] xsm: misra rule 8.4 fix

Stefano Stabellini posted 1 patch 1 year, 11 months ago
Failed in applying to current master (apply log)
[PATCH] xsm: misra rule 8.4 fix
Posted by Stefano Stabellini 1 year, 11 months ago 
Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
visible when an object or function with external linkage is defined")
found by cppcheck affecting xen/xsm/flask/ss/services.c.

Fix the first issue by making policydb_loaded_version static and the
second issue by declaring ss_initialized in a proper header.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index d319466c6b..b866e8d05f 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -56,8 +56,6 @@ static int bool_num = 0;
 static int *bool_pending_values = NULL;
 static int flask_security_make_bools(void);
 
-extern int ss_initialized;
-
 static int __init cf_check parse_flask_param(const char *s)
 {
     if ( !strcmp(s, "enforcing") )
diff --git a/xen/xsm/flask/private.h b/xen/xsm/flask/private.h
index 429f213cce..2e0e65489c 100644
--- a/xen/xsm/flask/private.h
+++ b/xen/xsm/flask/private.h
@@ -6,4 +6,6 @@
 long cf_check do_flask_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op);
 int cf_check compat_flask_op(XEN_GUEST_HANDLE_PARAM(void) u_flask_op);
 
+extern int ss_initialized;
+
 #endif /* XSM_FLASK_PRIVATE */
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index dab07b5f60..4665f3b007 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -53,7 +53,7 @@
 #include "conditional.h"
 #include "mls.h"
 
-unsigned int policydb_loaded_version;
+static unsigned int policydb_loaded_version;
 
 static DEFINE_RWLOCK(policy_rwlock);
 #define POLICY_RDLOCK read_lock(&policy_rwlock)
Re: [PATCH] xsm: misra rule 8.4 fix
Posted by Michal Orzel 1 year, 11 months ago 
Hi Stefano,

On 07/12/2022 03:12, Stefano Stabellini wrote:
> 
> 
> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
> visible when an object or function with external linkage is defined")
> found by cppcheck affecting xen/xsm/flask/ss/services.c.
> 
> Fix the first issue by making policydb_loaded_version static and the
> second issue by declaring ss_initialized in a proper header.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

cppcheck also reports findings for rule 8.4 with regards to the following functions:
- security_get_bools
- security_set_bools
- security_get_bool_value
- security_get_bool_name

The prototypes for them are stored in xen/xsm/flask/include/conditional.h,
but services.c only does:
#include "conditional.h"

This include refers to xen/xsm/flask/ss/conditional.h and not to xen/xsm/flask/include/conditional.h.
This means that we should also explicitly do:
#include <conditional.h>

This way we will fix all the findings in this file for the rule 8.4.

~Michal
Re: [PATCH] xsm: misra rule 8.4 fix
Posted by Jan Beulich 1 year, 11 months ago 
On 07.12.2022 13:33, Michal Orzel wrote:
> Hi Stefano,
> 
> On 07/12/2022 03:12, Stefano Stabellini wrote:
>>
>>
>> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
>> visible when an object or function with external linkage is defined")
>> found by cppcheck affecting xen/xsm/flask/ss/services.c.
>>
>> Fix the first issue by making policydb_loaded_version static and the
>> second issue by declaring ss_initialized in a proper header.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> cppcheck also reports findings for rule 8.4 with regards to the following functions:
> - security_get_bools
> - security_set_bools
> - security_get_bool_value
> - security_get_bool_name
> 
> The prototypes for them are stored in xen/xsm/flask/include/conditional.h,
> but services.c only does:
> #include "conditional.h"
> 
> This include refers to xen/xsm/flask/ss/conditional.h and not to xen/xsm/flask/include/conditional.h.
> This means that we should also explicitly do:
> #include <conditional.h>

And Misra has no rule disallowing such use of two different, identically
named headers, for being potentially ambiguous?

Jan
Re: [PATCH] xsm: misra rule 8.4 fix
Posted by Jan Beulich 1 year, 11 months ago 
On 08.12.2022 09:14, Jan Beulich wrote:
> On 07.12.2022 13:33, Michal Orzel wrote:
>> On 07/12/2022 03:12, Stefano Stabellini wrote:
>>> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
>>> visible when an object or function with external linkage is defined")
>>> found by cppcheck affecting xen/xsm/flask/ss/services.c.
>>>
>>> Fix the first issue by making policydb_loaded_version static and the
>>> second issue by declaring ss_initialized in a proper header.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>
>> cppcheck also reports findings for rule 8.4 with regards to the following functions:
>> - security_get_bools
>> - security_set_bools
>> - security_get_bool_value
>> - security_get_bool_name
>>
>> The prototypes for them are stored in xen/xsm/flask/include/conditional.h,
>> but services.c only does:
>> #include "conditional.h"
>>
>> This include refers to xen/xsm/flask/ss/conditional.h and not to xen/xsm/flask/include/conditional.h.
>> This means that we should also explicitly do:
>> #include <conditional.h>
> 
> And Misra has no rule disallowing such use of two different, identically
> named headers, for being potentially ambiguous?

Actually this is even more fragile than I thought when sending the above
question, albeit still working correctly as per gcc implementation defined
behavior. The further weakness stems from

CFLAGS-y += -iquote $(objtree)/xsm/flask/include
CFLAGS-y += -I$(srctree)/xsm/flask/include

which was added for out-of-tree builds, rendering in-tree builds search for
#include "..." files in xsm/flask/include too early. The only thing that
saves us is that the current directory is searched yet earlier.

However, as per gcc implementation defined behavior "#include <conditional.h>"
is likely wrong to use anyway, as this header can in no way be considered a
"system header"; it clearly falls under "header files of your own program",
where "own program" is Flask here. For being a system header the file ought
to live in include/xen/ or include/xsm/ (and accordingly be included via
"#include <xen/conditional.h>" or "#include <xsm/conditional.h>"), potentially
in a respective subdir there. My view is that these "#include <...>" (there
are more, albeit non-ambiguous ones) all want to be converted to
'#include "..."'. That'll then also eliminate the ambiguity with conditional.h
(as one will then [need to] come with a path prefix).

Jan
Re: [PATCH] xsm: misra rule 8.4 fix
Posted by Michal Orzel 1 year, 11 months ago 

On 08/12/2022 09:14, Jan Beulich wrote:
> 
> 
> On 07.12.2022 13:33, Michal Orzel wrote:
>> Hi Stefano,
>>
>> On 07/12/2022 03:12, Stefano Stabellini wrote:
>>>
>>>
>>> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
>>> visible when an object or function with external linkage is defined")
>>> found by cppcheck affecting xen/xsm/flask/ss/services.c.
>>>
>>> Fix the first issue by making policydb_loaded_version static and the
>>> second issue by declaring ss_initialized in a proper header.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>
>> cppcheck also reports findings for rule 8.4 with regards to the following functions:
>> - security_get_bools
>> - security_set_bools
>> - security_get_bool_value
>> - security_get_bool_name
>>
>> The prototypes for them are stored in xen/xsm/flask/include/conditional.h,
>> but services.c only does:
>> #include "conditional.h"
>>
>> This include refers to xen/xsm/flask/ss/conditional.h and not to xen/xsm/flask/include/conditional.h.
>> This means that we should also explicitly do:
>> #include <conditional.h>
> 
> And Misra has no rule disallowing such use of two different, identically
> named headers, for being potentially ambiguous?
I could not find such rule/directive related to filenames; only \wrt identifiers.
But all in all, we do not need MISRA to modify the filenames to get rid of ambiguity
if we really want to.

> 
> Jan

~Michal
Re: [PATCH] xsm: misra rule 8.4 fix
Posted by Jan Beulich 1 year, 11 months ago 
On 07.12.2022 03:12, Stefano Stabellini wrote:
> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
> visible when an object or function with external linkage is defined")
> found by cppcheck affecting xen/xsm/flask/ss/services.c.
> 
> Fix the first issue by making policydb_loaded_version static

This variable is only ever written to afaics, so perhaps better simply
drop it?

> and the
> second issue by declaring ss_initialized in a proper header.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

As to the title: The changes are contained to Flask, so xsm: really
is too wide a prefix.

> --- a/xen/xsm/flask/flask_op.c
> +++ b/xen/xsm/flask/flask_op.c
> @@ -56,8 +56,6 @@ static int bool_num = 0;
>  static int *bool_pending_values = NULL;
>  static int flask_security_make_bools(void);
>  
> -extern int ss_initialized;

What about the 2nd one in flask/ss/policydb.c?

Jan
Re: [PATCH] xsm: misra rule 8.4 fix
Posted by Daniel P. Smith 1 year, 11 months ago 
On 12/7/22 04:47, Jan Beulich wrote:
> On 07.12.2022 03:12, Stefano Stabellini wrote:
>> Fix two MISRA Issues Rule 8.4 ("A compatible declaration shall be
>> visible when an object or function with external linkage is defined")
>> found by cppcheck affecting xen/xsm/flask/ss/services.c.
>>
>> Fix the first issue by making policydb_loaded_version static
> 
> This variable is only ever written to afaics, so perhaps better simply
> drop it?

I agree, I would ack a patch with this dropped.

>> and the
>> second issue by declaring ss_initialized in a proper header.
>>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> As to the title: The changes are contained to Flask, so xsm: really
> is too wide a prefix.
> 
>> --- a/xen/xsm/flask/flask_op.c
>> +++ b/xen/xsm/flask/flask_op.c
>> @@ -56,8 +56,6 @@ static int bool_num = 0;
>>   static int *bool_pending_values = NULL;
>>   static int flask_security_make_bools(void);
>>   
>> -extern int ss_initialized;
> 
> What about the 2nd one in flask/ss/policydb.c?
> 
> Jan


dps

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.