| Message ID | 97008d1b28eb922b3c0041830b09e827396aa0ec.1711621080.git.simone.ballarin@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: address violations of MISRA C Rule 17.1 | expand |
On 28.03.2024 11:29, Simone Ballarin wrote: > The Xen community wants to avoid using variadic functions except for > specific circumstances where it feels appropriate by strict code review. In the title, s/20.7/17.1/ I suppose? Jan > Functions hypercall_create_continuation and hypercall_xlat_continuation > are internal helper functions 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> > > --- > Changes in v3: > - rebase: change SAF-3-safe to SAF-4-safe. > Changes in v2: > - replaced "special hypercalls" with "internal helper functions". > --- > docs/misra/safe.json | 8 ++++++++ > xen/arch/arm/domain.c | 1 + > xen/arch/x86/hypercall.c | 2 ++ > 3 files changed, 11 insertions(+) > > diff --git a/docs/misra/safe.json b/docs/misra/safe.json > index d361d0e65c..fe2bc18509 100644 > --- a/docs/misra/safe.json > +++ b/docs/misra/safe.json > @@ -36,6 +36,14 @@ > }, > { > "id": "SAF-4-safe", > + "analyser": { > + "eclair": "MC3R1.R17.1" > + }, > + "name": "Rule 17.1: internal helper functions 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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */ > va_list args; > > ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
On 28/03/24 11:31, Jan Beulich wrote: > On 28.03.2024 11:29, Simone Ballarin wrote: >> The Xen community wants to avoid using variadic functions except for >> specific circumstances where it feels appropriate by strict code review. > > In the title, s/20.7/17.1/ I suppose? > > Jan > >> Functions hypercall_create_continuation and hypercall_xlat_continuation >> are internal helper functions 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> >> >> --- >> Changes in v3: >> - rebase: change SAF-3-safe to SAF-4-safe. >> Changes in v2: >> - replaced "special hypercalls" with "internal helper functions". >> --- >> docs/misra/safe.json | 8 ++++++++ >> xen/arch/arm/domain.c | 1 + >> xen/arch/x86/hypercall.c | 2 ++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/docs/misra/safe.json b/docs/misra/safe.json >> index d361d0e65c..fe2bc18509 100644 >> --- a/docs/misra/safe.json >> +++ b/docs/misra/safe.json >> @@ -36,6 +36,14 @@ >> }, >> { >> "id": "SAF-4-safe", >> + "analyser": { >> + "eclair": "MC3R1.R17.1" >> + }, >> + "name": "Rule 17.1: internal helper functions 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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */ >> va_list args; >> >> ASSERT(nr <= ARRAY_SIZE(mcs->call.args)); > > Yes, sorry.
On Thu, 28 Mar 2024, Simone Ballarin wrote: > 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 internal helper functions 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> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> I fixed the title on commit > --- > Changes in v3: > - rebase: change SAF-3-safe to SAF-4-safe. > Changes in v2: > - replaced "special hypercalls" with "internal helper functions". > --- > docs/misra/safe.json | 8 ++++++++ > xen/arch/arm/domain.c | 1 + > xen/arch/x86/hypercall.c | 2 ++ > 3 files changed, 11 insertions(+) > > diff --git a/docs/misra/safe.json b/docs/misra/safe.json > index d361d0e65c..fe2bc18509 100644 > --- a/docs/misra/safe.json > +++ b/docs/misra/safe.json > @@ -36,6 +36,14 @@ > }, > { > "id": "SAF-4-safe", > + "analyser": { > + "eclair": "MC3R1.R17.1" > + }, > + "name": "Rule 17.1: internal helper functions 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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */ > va_list args; > > ASSERT(nr <= ARRAY_SIZE(mcs->call.args)); > -- > 2.34.1 >
On 05.04.2024 02:27, Stefano Stabellini wrote: > On Thu, 28 Mar 2024, Simone Ballarin wrote: >> 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 internal helper functions 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> > > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > I fixed the title on commit Did you forget to push then? I don't see this change in the tree, yet. Jan
On Fri, 5 Apr 2024, Jan Beulich wrote: > On 05.04.2024 02:27, Stefano Stabellini wrote: > > On Thu, 28 Mar 2024, Simone Ballarin wrote: > >> 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 internal helper functions 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> > > > > > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > > > I fixed the title on commit > > Did you forget to push then? I don't see this change in the tree, yet. Yep, thanks!
diff --git a/docs/misra/safe.json b/docs/misra/safe.json index d361d0e65c..fe2bc18509 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -36,6 +36,14 @@ }, { "id": "SAF-4-safe", + "analyser": { + "eclair": "MC3R1.R17.1" + }, + "name": "Rule 17.1: internal helper functions 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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */ va_list args; ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
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 internal helper functions 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> --- Changes in v3: - rebase: change SAF-3-safe to SAF-4-safe. Changes in v2: - replaced "special hypercalls" with "internal helper functions". --- docs/misra/safe.json | 8 ++++++++ xen/arch/arm/domain.c | 1 + xen/arch/x86/hypercall.c | 2 ++ 3 files changed, 11 insertions(+)