06 Mar, 2020

1 commit

  • The tests for initializing a variable defined between a switch
    statement's test and its first "case" statement are currently not
    initialized in Clang[1] nor the proposed auto-initialization feature in
    GCC.

    We should retain the test (so that we can evaluate compiler fixes),
    but mark it as an "expected fail". The rest of the kernel source will
    be adjusted to avoid this corner case.

    Also disable -Wswitch-unreachable for the test so that the intentionally
    broken code won't trigger warnings for GCC (nor future Clang) when
    initialization happens this unhandled place.

    [1] https://bugs.llvm.org/show_bug.cgi?id=44916

    Suggested-by: Alexander Potapenko
    Signed-off-by: Kees Cook
    [adelva: cherry picking to avoid boot test flakes]
    Bug: 144999193
    Link: https://lore.kernel.org/lkml/202002191358.2897A07C6@keescook/
    Change-Id: I0e691f2299ab42526ea306a92551a1188c469136
    Signed-off-by: Alistair Delva

    Kees Cook
     

05 Jun, 2019

1 commit

  • While the gcc plugin for automatic stack variable initialization (i.e.
    CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL) performs initialization with
    0x00 bytes, the Clang automatic stack variable initialization (i.e.
    CONFIG_INIT_STACK_ALL) uses various type-specific patterns that are
    typically 0xAA. Therefore the stackinit selftest has been fixed to check
    that bytes are no longer the test fill pattern of 0xFF (instead of looking
    for bytes that have become 0x00). This retains the test coverage for the
    0x00 pattern of the gcc plugin while adding coverage for the mostly 0xAA
    pattern of Clang.

    Signed-off-by: Kees Cook
    Acked-by: Ard Biesheuvel

    Kees Cook
     

02 Jun, 2019

1 commit

  • Prior to the adoption of SPDX, it was difficult for tools to determine
    the correct license due to incomplete or badly formatted license text.
    The SPDX solves this issue, assuming people can correctly spell
    "SPDX-License-Identifier" although this assumption is broken in some
    places.

    Since scripts/spdxcheck.py parses only lines that exactly matches to
    the correct tag, it cannot (should not) detect this kind of error.

    If the correct tag is missing, scripts/checkpatch.pl warns like this:

    WARNING: Missing or malformed SPDX-License-Identifier tag in line *

    So, people should notice it before the patch submission, but in reality
    broken tags sometimes slip in. The checkpatch warning is not useful for
    checking the committed files globally since large number of files still
    have no SPDX tag.

    Also, I am not sure about the legal effect when the SPDX tag is broken.

    Anyway, these typos are absolutely worth fixing. It is pretty easy to
    find suspicious lines by grep.

    $ git grep --not -e SPDX-License-Identifier --and -e SPDX- -- \
    :^LICENSES :^scripts/spdxcheck.py :^*/license-rules.rst
    arch/arm/kernel/bugs.c:// SPDX-Identifier: GPL-2.0
    drivers/phy/st/phy-stm32-usbphyc.c:// SPDX-Licence-Identifier: GPL-2.0
    drivers/pinctrl/sh-pfc/pfc-r8a77980.c:// SPDX-Lincense-Identifier: GPL 2.0
    lib/test_stackinit.c:// SPDX-Licenses: GPLv2
    sound/soc/codecs/max9759.c:// SPDX-Licence-Identifier: GPL-2.0

    Signed-off-by: Masahiro Yamada
    Signed-off-by: Greg Kroah-Hartman

    Masahiro Yamada
     

05 Mar, 2019

1 commit

  • Adds test for stack initialization coverage. We have several build options
    that control the level of stack variable initialization. This test lets us
    visualize which options cover which cases, and provide tests for some of
    the pathological padding conditions the compiler will sometimes fail to
    initialize.

    All options pass the explicit initialization cases and the partial
    initializers (even with padding):

    test_stackinit: u8_zero ok
    test_stackinit: u16_zero ok
    test_stackinit: u32_zero ok
    test_stackinit: u64_zero ok
    test_stackinit: char_array_zero ok
    test_stackinit: small_hole_zero ok
    test_stackinit: big_hole_zero ok
    test_stackinit: trailing_hole_zero ok
    test_stackinit: packed_zero ok
    test_stackinit: small_hole_dynamic_partial ok
    test_stackinit: big_hole_dynamic_partial ok
    test_stackinit: trailing_hole_dynamic_partial ok
    test_stackinit: packed_dynamic_partial ok
    test_stackinit: small_hole_static_partial ok
    test_stackinit: big_hole_static_partial ok
    test_stackinit: trailing_hole_static_partial ok
    test_stackinit: packed_static_partial ok
    test_stackinit: packed_static_all ok
    test_stackinit: packed_dynamic_all ok
    test_stackinit: packed_runtime_all ok

    The results of the other tests (which contain no explicit initialization),
    change based on the build's configured compiler instrumentation.

    No options:

    test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
    test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
    test_stackinit: trailing_hole_static_all FAIL (uninit bytes: 7)
    test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
    test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
    test_stackinit: trailing_hole_dynamic_all FAIL (uninit bytes: 7)
    test_stackinit: small_hole_runtime_partial FAIL (uninit bytes: 23)
    test_stackinit: big_hole_runtime_partial FAIL (uninit bytes: 127)
    test_stackinit: trailing_hole_runtime_partial FAIL (uninit bytes: 24)
    test_stackinit: packed_runtime_partial FAIL (uninit bytes: 24)
    test_stackinit: small_hole_runtime_all FAIL (uninit bytes: 3)
    test_stackinit: big_hole_runtime_all FAIL (uninit bytes: 61)
    test_stackinit: trailing_hole_runtime_all FAIL (uninit bytes: 7)
    test_stackinit: u8_none FAIL (uninit bytes: 1)
    test_stackinit: u16_none FAIL (uninit bytes: 2)
    test_stackinit: u32_none FAIL (uninit bytes: 4)
    test_stackinit: u64_none FAIL (uninit bytes: 8)
    test_stackinit: char_array_none FAIL (uninit bytes: 16)
    test_stackinit: switch_1_none FAIL (uninit bytes: 8)
    test_stackinit: switch_2_none FAIL (uninit bytes: 8)
    test_stackinit: small_hole_none FAIL (uninit bytes: 24)
    test_stackinit: big_hole_none FAIL (uninit bytes: 128)
    test_stackinit: trailing_hole_none FAIL (uninit bytes: 32)
    test_stackinit: packed_none FAIL (uninit bytes: 32)
    test_stackinit: user FAIL (uninit bytes: 32)
    test_stackinit: failures: 25

    CONFIG_GCC_PLUGIN_STRUCTLEAK_USER=y
    This only tries to initialize structs with __user markings, so
    only the difference from above is now the "user" test passes:

    test_stackinit: small_hole_static_all FAIL (uninit bytes: 3)
    test_stackinit: big_hole_static_all FAIL (uninit bytes: 61)
    test_stackinit: trailing_hole_static_all FAIL (uninit bytes: 7)
    test_stackinit: small_hole_dynamic_all FAIL (uninit bytes: 3)
    test_stackinit: big_hole_dynamic_all FAIL (uninit bytes: 61)
    test_stackinit: trailing_hole_dynamic_all FAIL (uninit bytes: 7)
    test_stackinit: small_hole_runtime_partial FAIL (uninit bytes: 23)
    test_stackinit: big_hole_runtime_partial FAIL (uninit bytes: 127)
    test_stackinit: trailing_hole_runtime_partial FAIL (uninit bytes: 24)
    test_stackinit: packed_runtime_partial FAIL (uninit bytes: 24)
    test_stackinit: small_hole_runtime_all FAIL (uninit bytes: 3)
    test_stackinit: big_hole_runtime_all FAIL (uninit bytes: 61)
    test_stackinit: trailing_hole_runtime_all FAIL (uninit bytes: 7)
    test_stackinit: u8_none FAIL (uninit bytes: 1)
    test_stackinit: u16_none FAIL (uninit bytes: 2)
    test_stackinit: u32_none FAIL (uninit bytes: 4)
    test_stackinit: u64_none FAIL (uninit bytes: 8)
    test_stackinit: char_array_none FAIL (uninit bytes: 16)
    test_stackinit: switch_1_none FAIL (uninit bytes: 8)
    test_stackinit: switch_2_none FAIL (uninit bytes: 8)
    test_stackinit: small_hole_none FAIL (uninit bytes: 24)
    test_stackinit: big_hole_none FAIL (uninit bytes: 128)
    test_stackinit: trailing_hole_none FAIL (uninit bytes: 32)
    test_stackinit: packed_none FAIL (uninit bytes: 32)
    test_stackinit: user ok
    test_stackinit: failures: 24

    CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF=y
    This initializes all structures passed by reference (scalars and strings
    remain uninitialized):

    test_stackinit: small_hole_static_all ok
    test_stackinit: big_hole_static_all ok
    test_stackinit: trailing_hole_static_all ok
    test_stackinit: small_hole_dynamic_all ok
    test_stackinit: big_hole_dynamic_all ok
    test_stackinit: trailing_hole_dynamic_all ok
    test_stackinit: small_hole_runtime_partial ok
    test_stackinit: big_hole_runtime_partial ok
    test_stackinit: trailing_hole_runtime_partial ok
    test_stackinit: packed_runtime_partial ok
    test_stackinit: small_hole_runtime_all ok
    test_stackinit: big_hole_runtime_all ok
    test_stackinit: trailing_hole_runtime_all ok
    test_stackinit: u8_none FAIL (uninit bytes: 1)
    test_stackinit: u16_none FAIL (uninit bytes: 2)
    test_stackinit: u32_none FAIL (uninit bytes: 4)
    test_stackinit: u64_none FAIL (uninit bytes: 8)
    test_stackinit: char_array_none FAIL (uninit bytes: 16)
    test_stackinit: switch_1_none FAIL (uninit bytes: 8)
    test_stackinit: switch_2_none FAIL (uninit bytes: 8)
    test_stackinit: small_hole_none ok
    test_stackinit: big_hole_none ok
    test_stackinit: trailing_hole_none ok
    test_stackinit: packed_none ok
    test_stackinit: user ok
    test_stackinit: failures: 7

    CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL=y
    This initializes all variables, so it matches above with the scalars
    and arrays included:

    test_stackinit: small_hole_static_all ok
    test_stackinit: big_hole_static_all ok
    test_stackinit: trailing_hole_static_all ok
    test_stackinit: small_hole_dynamic_all ok
    test_stackinit: big_hole_dynamic_all ok
    test_stackinit: trailing_hole_dynamic_all ok
    test_stackinit: small_hole_runtime_partial ok
    test_stackinit: big_hole_runtime_partial ok
    test_stackinit: trailing_hole_runtime_partial ok
    test_stackinit: packed_runtime_partial ok
    test_stackinit: small_hole_runtime_all ok
    test_stackinit: big_hole_runtime_all ok
    test_stackinit: trailing_hole_runtime_all ok
    test_stackinit: u8_none ok
    test_stackinit: u16_none ok
    test_stackinit: u32_none ok
    test_stackinit: u64_none ok
    test_stackinit: char_array_none ok
    test_stackinit: switch_1_none ok
    test_stackinit: switch_2_none ok
    test_stackinit: small_hole_none ok
    test_stackinit: big_hole_none ok
    test_stackinit: trailing_hole_none ok
    test_stackinit: packed_none ok
    test_stackinit: user ok
    test_stackinit: all tests passed!

    Signed-off-by: Kees Cook
    Reviewed-by: Ard Biesheuvel

    Kees Cook