From d8401f504b49c71280e504e41b3b56876094f081 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 4 Mar 2020 23:03:47 -0800 Subject: docs: deprecated.rst: Add %p to the list Once in a while %p usage comes up, and I've needed to have a reference to point people to. Add %p details to deprecated.rst. Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/202003042301.F844A8C0EC@keescook Signed-off-by: Jonathan Corbet --- Documentation/process/deprecated.rst | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'Documentation/process/deprecated.rst') diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst index 179f2a5625a0..7160a449e6c6 100644 --- a/Documentation/process/deprecated.rst +++ b/Documentation/process/deprecated.rst @@ -109,6 +109,28 @@ the given limit of bytes to copy. This is inefficient and can lead to linear read overflows if a source string is not NUL-terminated. The safe replacement is :c:func:`strscpy`. +%p format specifier +------------------- +Traditionally, using "%p" in format strings would lead to regular address +exposure flaws in dmesg, proc, sysfs, etc. Instead of leaving these to +be exploitable, all "%p" uses in the kernel are being printed as a hashed +value, rendering them unusable for addressing. New uses of "%p" should not +be added to the kernel. For text addresses, using "%pS" is likely better, +as it produces the more useful symbol name instead. For nearly everything +else, just do not add "%p" at all. + +Paraphrasing Linus's current `guidance `_: + +- If the hashed "%p" value is pointless, ask yourself whether the pointer + itself is important. Maybe it should be removed entirely? +- If you really think the true pointer value is important, why is some + system state or user privilege level considered "special"? If you think + you can justify it (in comments and commit log) well enough to stand + up to Linus's scrutiny, maybe you can use "%px", along with making sure + you have sensible permissions. + +And finally, know that a toggle for "%p" hashing will `not be accepted `_. + Variable Length Arrays (VLAs) ----------------------------- Using stack VLAs produces much worse machine code than statically -- cgit v1.2.3 From 76136e028d3bc94a84f5404ba0a9afae38db1b8a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 4 Mar 2020 11:03:24 -0800 Subject: docs: deprecated.rst: Clean up fall-through details 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 Acked-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/202003041102.47A4E4B62@keescook Signed-off-by: Jonathan Corbet --- Documentation/process/deprecated.rst | 48 ++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 19 deletions(-) (limited to 'Documentation/process/deprecated.rst') diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst index 7160a449e6c6..8965446f0b71 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 @@ -144,27 +144,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 `_, 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 -`_ - -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__)) +`_. +(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