* [PATCH] docs: deprecated.rst: Clean up fall-through details
@ 2020-03-04 19:03 Kees Cook
2020-03-04 19:30 ` Gustavo A. R. Silva
2020-03-10 17:23 ` Jonathan Corbet
0 siblings, 2 replies; 4+ messages in thread
From: Kees Cook @ 2020-03-04 19:03 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Joe Perches, Federico Vaga, Gustavo A. R. Silva,
Nick Desaulniers, linux-doc, linux-kernel
Add example of fall-through, list-ify the case ending statements, and
adjust the markup for links and readability. While here, adjust
strscpy() details to mention strscpy_pad().
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Documentation/process/deprecated.rst | 48 +++++++++++++++++-----------
1 file changed, 29 insertions(+), 19 deletions(-)
diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
index 179f2a5625a0..f9f196d3a69b 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -94,8 +94,8 @@ and other misbehavior due to the missing termination. It also NUL-pads the
destination buffer if the source contents are shorter than the destination
buffer size, which may be a needless performance penalty for callers using
only NUL-terminated strings. The safe replacement is :c:func:`strscpy`.
-(Users of :c:func:`strscpy` still needing NUL-padding will need an
-explicit :c:func:`memset` added.)
+(Users of :c:func:`strscpy` still needing NUL-padding should instead
+use strscpy_pad().)
If a caller is using non-NUL-terminated strings, :c:func:`strncpy()` can
still be used, but destinations should be marked with the `__nonstring
@@ -122,27 +122,37 @@ memory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`)
Implicit switch case fall-through
---------------------------------
-The C language allows switch cases to "fall-through" when a "break" statement
-is missing at the end of a case. This, however, introduces ambiguity in the
-code, as it's not always clear if the missing break is intentional or a bug.
+The C language allows switch cases to fall through to the next case
+when a "break" statement is missing at the end of a case. This, however,
+introduces ambiguity in the code, as it's not always clear if the missing
+break is intentional or a bug. For example, it's not obvious just from
+looking at the code if `STATE_ONE` is intentionally designed to fall
+through into `STATE_TWO`::
+
+ switch (value) {
+ case STATE_ONE:
+ do_something();
+ case STATE_TWO:
+ do_other();
+ break;
+ default:
+ WARN("unknown state");
+ }
As there have been a long list of flaws `due to missing "break" statements
<https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow
-"implicit fall-through".
-
-In order to identify intentional fall-through cases, we have adopted a
-pseudo-keyword macro 'fallthrough' which expands to gcc's extension
-__attribute__((__fallthrough__)). `Statement Attributes
-<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_
-
-When the C17/C18 [[fallthrough]] syntax is more commonly supported by
+implicit fall-through. In order to identify intentional fall-through
+cases, we have adopted a pseudo-keyword macro "fallthrough" which
+expands to gcc's extension `__attribute__((__fallthrough__))
+<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_.
+(When the C17/C18 `[[fallthrough]]` syntax is more commonly supported by
C compilers, static analyzers, and IDEs, we can switch to using that syntax
-for the macro pseudo-keyword.
+for the macro pseudo-keyword.)
All switch/case blocks must end in one of:
- break;
- fallthrough;
- continue;
- goto <label>;
- return [expression];
+* break;
+* fallthrough;
+* continue;
+* goto <label>;
+* return [expression];
--
2.20.1
--
Kees Cook
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Clean up fall-through details
2020-03-04 19:03 [PATCH] docs: deprecated.rst: Clean up fall-through details Kees Cook
@ 2020-03-04 19:30 ` Gustavo A. R. Silva
2020-03-10 17:23 ` Jonathan Corbet
1 sibling, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-04 19:30 UTC (permalink / raw)
To: Kees Cook, Jonathan Corbet
Cc: Joe Perches, Federico Vaga, Nick Desaulniers, linux-doc, linux-kernel
On 3/4/20 13:03, Kees Cook wrote:
> Add example of fall-through, list-ify the case ending statements, and
> adjust the markup for links and readability. While here, adjust
> strscpy() details to mention strscpy_pad().
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> Documentation/process/deprecated.rst | 48 +++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
> index 179f2a5625a0..f9f196d3a69b 100644
> --- a/Documentation/process/deprecated.rst
> +++ b/Documentation/process/deprecated.rst
> @@ -94,8 +94,8 @@ and other misbehavior due to the missing termination. It also NUL-pads the
> destination buffer if the source contents are shorter than the destination
> buffer size, which may be a needless performance penalty for callers using
> only NUL-terminated strings. The safe replacement is :c:func:`strscpy`.
> -(Users of :c:func:`strscpy` still needing NUL-padding will need an
> -explicit :c:func:`memset` added.)
> +(Users of :c:func:`strscpy` still needing NUL-padding should instead
> +use strscpy_pad().)
>
> If a caller is using non-NUL-terminated strings, :c:func:`strncpy()` can
> still be used, but destinations should be marked with the `__nonstring
> @@ -122,27 +122,37 @@ memory adjacent to the stack (when built without `CONFIG_VMAP_STACK=y`)
>
> Implicit switch case fall-through
> ---------------------------------
> -The C language allows switch cases to "fall-through" when a "break" statement
> -is missing at the end of a case. This, however, introduces ambiguity in the
> -code, as it's not always clear if the missing break is intentional or a bug.
> +The C language allows switch cases to fall through to the next case
> +when a "break" statement is missing at the end of a case. This, however,
> +introduces ambiguity in the code, as it's not always clear if the missing
> +break is intentional or a bug. For example, it's not obvious just from
> +looking at the code if `STATE_ONE` is intentionally designed to fall
> +through into `STATE_TWO`::
> +
> + switch (value) {
> + case STATE_ONE:
> + do_something();
> + case STATE_TWO:
> + do_other();
> + break;
> + default:
> + WARN("unknown state");
> + }
>
> As there have been a long list of flaws `due to missing "break" statements
> <https://cwe.mitre.org/data/definitions/484.html>`_, we no longer allow
> -"implicit fall-through".
> -
> -In order to identify intentional fall-through cases, we have adopted a
> -pseudo-keyword macro 'fallthrough' which expands to gcc's extension
> -__attribute__((__fallthrough__)). `Statement Attributes
> -<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_
> -
> -When the C17/C18 [[fallthrough]] syntax is more commonly supported by
> +implicit fall-through. In order to identify intentional fall-through
> +cases, we have adopted a pseudo-keyword macro "fallthrough" which
> +expands to gcc's extension `__attribute__((__fallthrough__))
> +<https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_.
> +(When the C17/C18 `[[fallthrough]]` syntax is more commonly supported by
> C compilers, static analyzers, and IDEs, we can switch to using that syntax
> -for the macro pseudo-keyword.
> +for the macro pseudo-keyword.)
>
> All switch/case blocks must end in one of:
>
> - break;
> - fallthrough;
> - continue;
> - goto <label>;
> - return [expression];
> +* break;
> +* fallthrough;
> +* continue;
> +* goto <label>;
> +* return [expression];
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Clean up fall-through details
2020-03-04 19:03 [PATCH] docs: deprecated.rst: Clean up fall-through details Kees Cook
2020-03-04 19:30 ` Gustavo A. R. Silva
@ 2020-03-10 17:23 ` Jonathan Corbet
2020-03-10 19:22 ` Kees Cook
1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Corbet @ 2020-03-10 17:23 UTC (permalink / raw)
To: Kees Cook
Cc: Joe Perches, Federico Vaga, Gustavo A. R. Silva,
Nick Desaulniers, linux-doc, linux-kernel
On Wed, 4 Mar 2020 11:03:24 -0800
Kees Cook <keescook@chromium.org> wrote:
> Add example of fall-through, list-ify the case ending statements, and
> adjust the markup for links and readability. While here, adjust
> strscpy() details to mention strscpy_pad().
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Applied, thanks. But ...
> ---
> Documentation/process/deprecated.rst | 48 +++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
> index 179f2a5625a0..f9f196d3a69b 100644
> --- a/Documentation/process/deprecated.rst
> +++ b/Documentation/process/deprecated.rst
> @@ -94,8 +94,8 @@ and other misbehavior due to the missing termination. It also NUL-pads the
> destination buffer if the source contents are shorter than the destination
> buffer size, which may be a needless performance penalty for callers using
> only NUL-terminated strings. The safe replacement is :c:func:`strscpy`.
> -(Users of :c:func:`strscpy` still needing NUL-padding will need an
> -explicit :c:func:`memset` added.)
> +(Users of :c:func:`strscpy` still needing NUL-padding should instead
> +use strscpy_pad().)
:c:func: usage should really be stomped on when we encounter it. There's
a few in this file; I'll tack on a quick patch making them go away.
jon
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] docs: deprecated.rst: Clean up fall-through details
2020-03-10 17:23 ` Jonathan Corbet
@ 2020-03-10 19:22 ` Kees Cook
0 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2020-03-10 19:22 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Joe Perches, Federico Vaga, Gustavo A. R. Silva,
Nick Desaulniers, linux-doc, linux-kernel
On Tue, Mar 10, 2020 at 11:23:56AM -0600, Jonathan Corbet wrote:
> On Wed, 4 Mar 2020 11:03:24 -0800
> Kees Cook <keescook@chromium.org> wrote:
>
> > Add example of fall-through, list-ify the case ending statements, and
> > adjust the markup for links and readability. While here, adjust
> > strscpy() details to mention strscpy_pad().
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Applied, thanks. But ...
>
> > ---
> > Documentation/process/deprecated.rst | 48 +++++++++++++++++-----------
> > 1 file changed, 29 insertions(+), 19 deletions(-)
> >
> > diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
> > index 179f2a5625a0..f9f196d3a69b 100644
> > --- a/Documentation/process/deprecated.rst
> > +++ b/Documentation/process/deprecated.rst
> > @@ -94,8 +94,8 @@ and other misbehavior due to the missing termination. It also NUL-pads the
> > destination buffer if the source contents are shorter than the destination
> > buffer size, which may be a needless performance penalty for callers using
> > only NUL-terminated strings. The safe replacement is :c:func:`strscpy`.
> > -(Users of :c:func:`strscpy` still needing NUL-padding will need an
> > -explicit :c:func:`memset` added.)
> > +(Users of :c:func:`strscpy` still needing NUL-padding should instead
> > +use strscpy_pad().)
>
> :c:func: usage should really be stomped on when we encounter it. There's
> a few in this file; I'll tack on a quick patch making them go away.
Oops, yes, I meant to do another pass for that. I will double-check
future patches!
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-10 19:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-04 19:03 [PATCH] docs: deprecated.rst: Clean up fall-through details Kees Cook
2020-03-04 19:30 ` Gustavo A. R. Silva
2020-03-10 17:23 ` Jonathan Corbet
2020-03-10 19:22 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).