| Message ID | a5137c812eefab7e0417670386b0fee35468504d.1718264354.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [XEN] automation/eclair: add deviation for MISRA C Rule 17.7 | expand |
On 13.06.2024 11:07, Federico Serafini wrote: > --- a/docs/misra/deviations.rst > +++ b/docs/misra/deviations.rst > @@ -364,6 +364,17 @@ Deviations related to MISRA C:2012 Rules: > by `stdarg.h`. > - Tagged as `deliberate` for ECLAIR. > > + * - R17.7 > + - Not using the return value of a function do not endanger safety if it > + coincides with the first actual argument. > + - Tagged as `safe` for ECLAIR. Such functions are: > + - __builtin_memcpy() > + - __builtin_memmove() > + - __builtin_memset() > + - __cpumask_check() > + - strlcat() > + - strlcpy() These last two aren't similar to strcat/strcpy in what they return, so I'm not convinced they should be listed here. Certainly not with the "coincides" justification. Jan
On 13/06/24 12:08, Jan Beulich wrote: > On 13.06.2024 11:07, Federico Serafini wrote: >> --- a/docs/misra/deviations.rst >> +++ b/docs/misra/deviations.rst >> @@ -364,6 +364,17 @@ Deviations related to MISRA C:2012 Rules: >> by `stdarg.h`. >> - Tagged as `deliberate` for ECLAIR. >> >> + * - R17.7 >> + - Not using the return value of a function do not endanger safety if it >> + coincides with the first actual argument. >> + - Tagged as `safe` for ECLAIR. Such functions are: >> + - __builtin_memcpy() >> + - __builtin_memmove() >> + - __builtin_memset() >> + - __cpumask_check() >> + - strlcat() >> + - strlcpy() > > These last two aren't similar to strcat/strcpy in what they return, so I'm > not convinced they should be listed here. Certainly not with the "coincides" > justification. Right, thanks.
On 13/06/24 12:08, Jan Beulich wrote: > On 13.06.2024 11:07, Federico Serafini wrote: >> --- a/docs/misra/deviations.rst >> +++ b/docs/misra/deviations.rst >> @@ -364,6 +364,17 @@ Deviations related to MISRA C:2012 Rules: >> by `stdarg.h`. >> - Tagged as `deliberate` for ECLAIR. >> >> + * - R17.7 >> + - Not using the return value of a function do not endanger safety if it >> + coincides with the first actual argument. >> + - Tagged as `safe` for ECLAIR. Such functions are: >> + - __builtin_memcpy() >> + - __builtin_memmove() >> + - __builtin_memset() >> + - __cpumask_check() >> + - strlcat() >> + - strlcpy() > > These last two aren't similar to strcat/strcpy in what they return, so I'm > not convinced they should be listed here. Certainly not with the "coincides" > justification. Thanks to violations of Rule 17.7 I noticed that safe_strcpy() and safe_strcat() are used without checking the return value. Is this intentional? [1] https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/665/PROJECT.ecd;/by_service/MC3R1.R17.7.html#{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":true,"selector":{"enabled":true,"negated":false,"kind":2,"children":[{"enabled":true,"negated":false,"kind":0,"domain":"message","inputs":[{"enabled":true,"text":"^.*safe_strcpy"}]}]}}} [2] https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/665/PROJECT.ecd;/sources/xen/arch/x86/setup.c.html#R5021_1{"select":true,"selection":{"hiddenAreaKinds":[],"hiddenSubareaKinds":[],"show":true,"selector":{"enabled":true,"negated":false,"kind":2,"children":[{"enabled":true,"negated":false,"kind":0,"domain":"message","inputs":[{"enabled":true,"text":"^.*safe_strcat"}]}]}}}
On 13.06.2024 13:50, Federico Serafini wrote: > On 13/06/24 12:08, Jan Beulich wrote: >> On 13.06.2024 11:07, Federico Serafini wrote: >>> --- a/docs/misra/deviations.rst >>> +++ b/docs/misra/deviations.rst >>> @@ -364,6 +364,17 @@ Deviations related to MISRA C:2012 Rules: >>> by `stdarg.h`. >>> - Tagged as `deliberate` for ECLAIR. >>> >>> + * - R17.7 >>> + - Not using the return value of a function do not endanger safety if it >>> + coincides with the first actual argument. >>> + - Tagged as `safe` for ECLAIR. Such functions are: >>> + - __builtin_memcpy() >>> + - __builtin_memmove() >>> + - __builtin_memset() >>> + - __cpumask_check() >>> + - strlcat() >>> + - strlcpy() >> >> These last two aren't similar to strcat/strcpy in what they return, so I'm >> not convinced they should be listed here. Certainly not with the "coincides" >> justification. > > Thanks to violations of Rule 17.7 I noticed that safe_strcpy() > and safe_strcat() are used without checking the return value. > Is this intentional? I expect that's case by case judgement. The main thing for them is to make sure the destination buffer isn't overrun. There may be callers which can live with possible truncation, there may be other callers which guarantee a suitably sized buffer, and there may also be callers which actually ought to check. Jan
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl index 447c1e6661..7bae804569 100644 --- a/automation/eclair_analysis/ECLAIR/deviations.ecl +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl @@ -413,6 +413,10 @@ explicit comment indicating the fallthrough intention is present." -config=MC3R1.R17.1,macros+={hide , "^va_(arg|start|copy|end)$"} -doc_end +-doc_begin="Not using the return value of a function do not endanger safety if it coincides with the first actual argument." +-config=MC3R1.R17.7,calls+={safe, "any()", "decl(name(__builtin_memcpy||__builtin_memmove||__builtin_memset||cpumask_check||strlcat||strlcpy))"} +-doc_end + # # Series 18. # diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index 36959aa44a..0bbac3cb9a 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -364,6 +364,17 @@ Deviations related to MISRA C:2012 Rules: by `stdarg.h`. - Tagged as `deliberate` for ECLAIR. + * - R17.7 + - Not using the return value of a function do not endanger safety if it + coincides with the first actual argument. + - Tagged as `safe` for ECLAIR. Such functions are: + - __builtin_memcpy() + - __builtin_memmove() + - __builtin_memset() + - __cpumask_check() + - strlcat() + - strlcpy() + * - R20.4 - The override of the keyword \"inline\" in xen/compiler.h is present so that section contents checks pass when the compiler chooses not to
Update ECLAIR configuration to deviate some cases where not using the return value of a function is not dangerous. Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++ docs/misra/deviations.rst | 11 +++++++++++ 2 files changed, 15 insertions(+)