| Message ID | f7c2f12ab1b62301cfea3a28707178950f480932.1710923235.git.simone.ballarin@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: address violations of MISRA C Rule 17.1 | expand |
On 20.03.2024 09:51, Simone Ballarin wrote: > MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" > > The Xen community wants to avoid using variadic functions except for > specific circumstances where it feels appropriate by strict code review. > > Functions hypercall_create_continuation and hypercall_xlat_continuation > are special hypercalls made to break long running hypercalls into multiple > calls. Here and below: These aren't "special hypercalls". They're internal helper functions. > They take a variable number of arguments depending on the original > hypercall they are trying to continue. Am I misremembering or did Andrew outline a plan to eliminate the variadic- ness from these? From certifiability perspective avoiding the need for a deviation would likely be preferable? Jan
On Wed, 20 Mar 2024, Jan Beulich wrote: > On 20.03.2024 09:51, Simone Ballarin wrote: > > MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" > > > > The Xen community wants to avoid using variadic functions except for > > specific circumstances where it feels appropriate by strict code review. > > > > Functions hypercall_create_continuation and hypercall_xlat_continuation > > are special hypercalls made to break long running hypercalls into multiple > > calls. > > Here and below: These aren't "special hypercalls". They're internal helper > functions. +1 > > They take a variable number of arguments depending on the original > > hypercall they are trying to continue. > > Am I misremembering or did Andrew outline a plan to eliminate the variadic- > ness from these? From certifiability perspective avoiding the need for a > deviation would likely be preferable? For sure, it would be preferable. In the meantime we can have the SAF comment?
On 21/03/24 02:47, Stefano Stabellini wrote: > On Wed, 20 Mar 2024, Jan Beulich wrote: >> On 20.03.2024 09:51, Simone Ballarin wrote: >>> MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" >>> >>> The Xen community wants to avoid using variadic functions except for >>> specific circumstances where it feels appropriate by strict code review. >>> >>> Functions hypercall_create_continuation and hypercall_xlat_continuation >>> are special hypercalls made to break long running hypercalls into multiple >>> calls. >> >> Here and below: These aren't "special hypercalls". They're internal helper >> functions. > > +1 > > >>> They take a variable number of arguments depending on the original >>> hypercall they are trying to continue. >> >> Am I misremembering or did Andrew outline a plan to eliminate the variadic- >> ness from these? From certifiability perspective avoiding the need for a >> deviation would likely be preferable? > > For sure, it would be preferable. In the meantime we can have the SAF > comment? I agree in using the SAF comments as a temporary measure. I'll propose a new patch with the fix requested by Jan.
diff --git a/docs/misra/safe.json b/docs/misra/safe.json index 952324f85c..65c90c7618 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -28,6 +28,14 @@ }, { "id": "SAF-3-safe", + "analyser": { + "eclair": "MC3R1.R17.1" + }, + "name": "Rule 17.1: special hypercall made to break long running hypercalls into multiple calls.", + "text": "They need to take a variable number of arguments depending on the original hypercall they are trying to continue." + }, + { + "id": "SAF-4-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 5e7a7f3e7e..f5706bd5b8 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -382,6 +382,7 @@ unsigned long hypercall_create_continuation( const char *p = format; unsigned long arg, rc; unsigned int i; + /* SAF-3-safe allowed variadic function */ va_list args; current->hcall_preempted = true; diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c index 01cd73040d..18d8c75522 100644 --- a/xen/arch/x86/hypercall.c +++ b/xen/arch/x86/hypercall.c @@ -31,6 +31,7 @@ unsigned long hypercall_create_continuation( const char *p = format; unsigned long arg; unsigned int i; + /* SAF-3-safe allowed variadic function */ va_list args; curr->hcall_preempted = true; @@ -115,6 +116,7 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int nr, struct cpu_user_regs *regs; unsigned int i, cval = 0; unsigned long nval = 0; + /* SAF-3-safe allowed variadic function */ va_list args; ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" The Xen community wants to avoid using variadic functions except for specific circumstances where it feels appropriate by strict code review. Functions hypercall_create_continuation and hypercall_xlat_continuation are special hypercalls made to break long running hypercalls into multiple calls. They take a variable number of arguments depending on the original hypercall they are trying to continue. Add SAF deviations for the aforementioned functions. Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> --- docs/misra/safe.json | 8 ++++++++ xen/arch/arm/domain.c | 1 + xen/arch/x86/hypercall.c | 2 ++ 3 files changed, 11 insertions(+)