1. 18 Oct, 2021 7 commits
    • Kees Cook's avatar
      stddef: Introduce DECLARE_FLEX_ARRAY() helper · 3080ea55
      Kees Cook authored
      There are many places where kernel code wants to have several different
      typed trailing flexible arrays. This would normally be done with multiple
      flexible arrays in a union, but since GCC and Clang don't (on the surface)
      allow this, there have been many open-coded workarounds, usually involving
      neighboring 0-element arrays at the end of a structure. For example,
      instead of something like this:
      
      struct thing {
      	...
      	union {
      		struct type1 foo[];
      		struct type2 bar[];
      	};
      };
      
      code works around the compiler with:
      
      struct thing {
      	...
      	struct type1 foo[0];
      	struct type2 bar[];
      };
      
      Another case is when a flexible array is wanted as the single member
      within a struct (which itself is usually in a union). For example, this
      would be worked around as:
      
      union many {
      	...
      	struct {
      		struct type3 baz[0];
      	};
      };
      
      These kinds of work-arounds cause problems with size checks against such
      zero-element arrays (for example when building with -Warray-bounds and
      -Wzero-length-bounds, and with the coming FORTIFY_SOURCE improvements),
      so they must all be converted to "real" flexible arrays, avoiding warnings
      like this:
      
      fs/hpfs/anode.c: In function 'hpfs_add_sector_to_btree':
      fs/hpfs/anode.c:209:27: warning: array subscript 0 is outside the bounds of an interior zero-length array 'struct bplus_internal_node[0]' [-Wzero-length-bounds]
        209 |    anode->btree.u.internal[0].down = cpu_to_le32(a);
            |    ~~~~~~~~~~~~~~~~~~~~~~~^~~
      In file included from fs/hpfs/hpfs_fn.h:26,
                       from fs/hpfs/anode.c:10:
      fs/hpfs/hpfs.h:412:32: note: while referencing 'internal'
        412 |     struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
            |                                ^~~~~~~~
      
      drivers/net/can/usb/etas_es58x/es58x_fd.c: In function 'es58x_fd_tx_can_msg':
      drivers/net/can/usb/etas_es58x/es58x_fd.c:360:35: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[]'} [-Wzero-length-bounds]
        360 |  tx_can_msg = (typeof(tx_can_msg))&es58x_fd_urb_cmd->raw_msg[msg_len];
            |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      In file included from drivers/net/can/usb/etas_es58x/es58x_core.h:22,
                       from drivers/net/can/usb/etas_es58x/es58x_fd.c:17:
      drivers/net/can/usb/etas_es58x/es58x_fd.h:231:6: note: while referencing 'raw_msg'
        231 |   u8 raw_msg[0];
            |      ^~~~~~~
      
      However, it _is_ entirely possible to have one or more flexible arrays
      in a struct or union: it just has to be in another struct. And since it
      cannot be alone in a struct, such a struct must have at least 1 other
      named member -- but that member can be zero sized. Wrap all this nonsense
      into the new DECLARE_FLEX_ARRAY() in support of having flexible arrays
      in unions (or alone in a struct).
      
      As with struct_group(), since this is needed in UAPI headers as well,
      implement the core there, with a non-UAPI wrapper.
      
      Additionally update kernel-doc to understand its existence.
      
      https://github.com/KSPP/linux/issues/137
      
      Cc: Arnd Bergmann <arnd@arndb.de>
      Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      3080ea55
    • Kees Cook's avatar
      btrfs: Use memset_startat() to clear end of struct · a2c5062f
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Use memset_startat() so memset() doesn't get confused about writing
      beyond the destination member that is intended to be the starting point
      of zeroing through the end of the struct.
      
      Cc: Chris Mason <clm@fb.com>
      Cc: Josef Bacik <josef@toxicpanda.com>
      Cc: David Sterba <dsterba@suse.com>
      Cc: linux-btrfs@vger.kernel.org
      Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
      Acked-by: default avatarDavid Sterba <dsterba@suse.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      a2c5062f
    • Kees Cook's avatar
      string.h: Introduce memset_startat() for wiping trailing members and padding · 6dbefad4
      Kees Cook authored
      A common idiom in kernel code is to wipe the contents of a structure
      starting from a given member. These open-coded cases are usually difficult
      to read and very sensitive to struct layout changes. Like memset_after(),
      introduce a new helper, memset_startat() that takes the target struct
      instance, the byte to write, and the member name where zeroing should
      start.
      
      Note that this doesn't zero padding preceding the target member. For
      those cases, memset_after() should be used on the preceding member.
      
      Cc: Steffen Klassert <steffen.klassert@secunet.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: Jakub Kicinski <kuba@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Francis Laniel <laniel_francis@privacyrequired.com>
      Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
      Cc: Daniel Axtens <dja@axtens.net>
      Cc: netdev@vger.kernel.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      6dbefad4
    • Kees Cook's avatar
      xfrm: Use memset_after() to clear padding · caf283d0
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Clear trailing padding bytes using the new helper so that memset()
      doesn't get confused about writing "past the end" of the last struct
      member. There is no change to the resulting machine code.
      
      Cc: Steffen Klassert <steffen.klassert@secunet.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: Jakub Kicinski <kuba@kernel.org>
      Cc: netdev@vger.kernel.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      caf283d0
    • Kees Cook's avatar
      string.h: Introduce memset_after() for wiping trailing members/padding · 4797632f
      Kees Cook authored
      A common idiom in kernel code is to wipe the contents of a structure
      after a given member. This is especially useful in places where there is
      trailing padding. These open-coded cases are usually difficult to read
      and very sensitive to struct layout changes. Introduce a new helper,
      memset_after() that takes the target struct instance, the byte to write,
      and the member name after which the zeroing should start.
      
      Cc: Steffen Klassert <steffen.klassert@secunet.com>
      Cc: Herbert Xu <herbert@gondor.apana.org.au>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: Jakub Kicinski <kuba@kernel.org>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Francis Laniel <laniel_francis@privacyrequired.com>
      Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
      Cc: Daniel Axtens <dja@axtens.net>
      Cc: netdev@vger.kernel.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      4797632f
    • Kees Cook's avatar
      lib: Introduce CONFIG_MEMCPY_KUNIT_TEST · bb95ebbe
      Kees Cook authored
      Before changing anything about memcpy(), memmove(), and memset(), add
      run-time tests to check basic behaviors for any regressions.
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      bb95ebbe
    • Kees Cook's avatar
      fortify: Add compile-time FORTIFY_SOURCE tests · be58f710
      Kees Cook authored
      While the run-time testing of FORTIFY_SOURCE is already present in
      LKDTM, there is no testing of the expected compile-time detections. In
      preparation for correctly supporting FORTIFY_SOURCE under Clang, adding
      additional FORTIFY_SOURCE defenses, and making sure FORTIFY_SOURCE
      doesn't silently regress with GCC, introduce a build-time test suite that
      checks each expected compile-time failure condition.
      
      As this is relatively backwards from standard build rules in the
      sense that a successful test is actually a compile _failure_, create
      a wrapper script to check for the correct errors, and wire it up as
      a dummy dependency to lib/string.o, collecting the results into a log
      file artifact.
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      be58f710
  2. 25 Sep, 2021 19 commits
    • Kees Cook's avatar
      fortify: Allow strlen() and strnlen() to pass compile-time known lengths · 3009f891
      Kees Cook authored
      Under CONFIG_FORTIFY_SOURCE, it is possible for the compiler to perform
      strlen() and strnlen() at compile-time when the string size is known.
      This is required to support compile-time overflow checking in strlcpy().
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      3009f891
    • Kees Cook's avatar
      fortify: Prepare to improve strnlen() and strlen() warnings · 369cd216
      Kees Cook authored
      In order to have strlen() use fortified strnlen() internally, swap their
      positions in the source. Doing this as part of later changes makes
      review difficult, so reoroder it here; no code changes.
      
      Cc: Francis Laniel <laniel_francis@privacyrequired.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      369cd216
    • Kees Cook's avatar
      fortify: Fix dropped strcpy() compile-time write overflow check · 072af0c6
      Kees Cook authored
      The implementation for intra-object overflow in str*-family functions
      accidentally dropped compile-time write overflow checking in strcpy(),
      leaving it entirely to run-time. Add back the intended check.
      
      Fixes: 6a39e62a ("lib: string.h: detect intra-object overflow in fortified string functions")
      Cc: Daniel Axtens <dja@axtens.net>
      Cc: Francis Laniel <laniel_francis@privacyrequired.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      072af0c6
    • Kees Cook's avatar
      fortify: Explicitly disable Clang support · a52f8a59
      Kees Cook authored
      Clang has never correctly compiled the FORTIFY_SOURCE defenses due to
      a couple bugs:
      
      	Eliding inlines with matching __builtin_* names
      	https://bugs.llvm.org/show_bug.cgi?id=50322
      
      	Incorrect __builtin_constant_p() of some globals
      	https://bugs.llvm.org/show_bug.cgi?id=41459
      
      In the process of making improvements to the FORTIFY_SOURCE defenses, the
      first (silent) bug (coincidentally) becomes worked around, but exposes
      the latter which breaks the build. As such, Clang must not be used with
      CONFIG_FORTIFY_SOURCE until at least latter bug is fixed (in Clang 13),
      and the fortify routines have been rearranged.
      
      Update the Kconfig to reflect the reality of the current situation.
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      Link: https://lore.kernel.org/lkml/CAKwvOd=A+ueGV2ihdy5GtgR2fQbcXjjAtVxv3=cPjffpebZB7A@mail.gmail.com
      a52f8a59
    • Kees Cook's avatar
      fortify: Move remaining fortify helpers into fortify-string.h · c430f600
      Kees Cook authored
      When commit a28a6e86 ("string.h: move fortified functions definitions
      in a dedicated header.") moved the fortify-specific code, some helpers
      were left behind. Move the remaining fortify-specific helpers into
      fortify-string.h so they're together where they're used. This requires
      that any FORTIFY helper function prototypes be conditionally built to
      avoid "no prototype" warnings. Additionally removes unused helpers.
      
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Daniel Axtens <dja@axtens.net>
      Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
      Cc: Andrey Konovalov <andreyknvl@google.com>
      Cc: Dan Williams <dan.j.williams@intel.com>
      Acked-by: default avatarFrancis Laniel <laniel_francis@privacyrequired.com>
      Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      c430f600
    • Kees Cook's avatar
      lib/string: Move helper functions out of string.c · cfecea6e
      Kees Cook authored
      The core functions of string.c are those that may be implemented by
      per-architecture functions, or overloaded by FORTIFY_SOURCE. As a
      result, it needs to be built with __NO_FORTIFY. Without this, macros
      will collide with function declarations. This was accidentally working
      due to -ffreestanding (on some architectures). Make this deterministic
      by explicitly setting __NO_FORTIFY and move all the helper functions
      into string_helpers.c so that they gain the fortification coverage they
      had been missing.
      
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Nick Desaulniers <ndesaulniers@google.com>
      Cc: Andy Lavr <andy.lavr@gmail.com>
      Cc: Nathan Chancellor <nathan@kernel.org>
      Cc: Alexey Dobriyan <adobriyan@gmail.com>
      Cc: Stephen Rothwell <sfr@canb.auug.org.au>
      Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
      Acked-by: default avatarAndy Shevchenko <andriy.shevchenko@linux.intel.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      cfecea6e
    • Kees Cook's avatar
      compiler_types.h: Remove __compiletime_object_size() · c80d92fb
      Kees Cook authored
      Since all compilers support __builtin_object_size(), and there is only
      one user of __compiletime_object_size, remove it to avoid the needless
      indirection. This lets Clang reason about check_copy_size() correctly.
      
      Link: https://github.com/ClangBuiltLinux/linux/issues/1179Suggested-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      Cc: Nathan Chancellor <nathan@kernel.org>
      Cc: Nick Desaulniers <ndesaulniers@google.com>
      Cc: Sedat Dilek <sedat.dilek@gmail.com>
      Cc: Will Deacon <will@kernel.org>
      Cc: Marco Elver <elver@google.com>
      Cc: Arvind Sankar <nivedita@alum.mit.edu>
      Cc: Masahiro Yamada <masahiroy@kernel.org>
      Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Sami Tolvanen <samitolvanen@google.com>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Gabriel Krisman Bertazi <krisman@collabora.com>
      Cc: Andy Lutomirski <luto@kernel.org>
      Cc: Oleg Nesterov <oleg@redhat.com>
      Reviewed-by: default avatarMiguel Ojeda <ojeda@kernel.org>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      c80d92fb
    • Kees Cook's avatar
      cm4000_cs: Use struct_group() to zero struct cm4000_dev region · 8610047c
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Add struct_group() to mark region of struct cm4000_dev that should be
      initialized to zero.
      
      Cc: Harald Welte <laforge@gnumonks.org>
      Cc: Arnd Bergmann <arnd@arndb.de>
      Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
      Link: https://lore.kernel.org/lkml/YQDvxAofJlI1JoGZ@kroah.com
      8610047c
    • Kees Cook's avatar
      can: flexcan: Use struct_group() to zero struct flexcan_regs regions · c92a08c1
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Add struct_group() to mark both regions of struct flexcan_regs that get
      initialized to zero. Avoid the future warnings:
      
      In function 'fortify_memset_chk',
          inlined from 'memset_io' at ./include/asm-generic/io.h:1169:2,
          inlined from 'flexcan_ram_init' at drivers/net/can/flexcan.c:1403:2:
      ./include/linux/fortify-string.h:199:4: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
        199 |    __write_overflow_field(p_size_field, size);
            |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      In function 'fortify_memset_chk',
          inlined from 'memset_io' at ./include/asm-generic/io.h:1169:2,
          inlined from 'flexcan_ram_init' at drivers/net/can/flexcan.c:1408:3:
      ./include/linux/fortify-string.h:199:4: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
        199 |    __write_overflow_field(p_size_field, size);
            |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      Cc: Wolfgang Grandegger <wg@grandegger.com>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: Jakub Kicinski <kuba@kernel.org>
      Cc: linux-can@vger.kernel.org
      Cc: netdev@vger.kernel.org
      Acked-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      c92a08c1
    • Kees Cook's avatar
      HID: roccat: Use struct_group() to zero kone_mouse_event · 69dae0fe
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Add struct_group() to mark region of struct kone_mouse_event that should
      be initialized to zero.
      
      Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
      Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
      Cc: linux-input@vger.kernel.org
      Acked-by: default avatarJiri Kosina <jikos@kernel.org>
      Link: https://lore.kernel.org/lkml/nycvar.YFH.7.76.2108201810560.15313@cbobk.fhfr.pmSigned-off-by: default avatarKees Cook <keescook@chromium.org>
      69dae0fe
    • Kees Cook's avatar
      HID: cp2112: Use struct_group() for memcpy() region · 5e423a0c
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memcpy(), memmove(), and memset(), avoid
      intentionally writing across neighboring fields.
      
      Use struct_group() in struct cp2112_string_report around members report,
      length, type, and string, so they can be referenced together. This will
      allow memcpy() and sizeof() to more easily reason about sizes, improve
      readability, and avoid future warnings about writing beyond the end of
      report.
      
      "pahole" shows no size nor member offset changes to struct
      cp2112_string_report.  "objdump -d" shows no meaningful object
      code changes (i.e. only source line number induced differences.)
      
      Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
      Cc: linux-input@vger.kernel.org
      Acked-by: default avatarJiri Kosina <jikos@kernel.org>
      Link: https://lore.kernel.org/lkml/nycvar.YFH.7.76.2108201810560.15313@cbobk.fhfr.pmSigned-off-by: default avatarKees Cook <keescook@chromium.org>
      5e423a0c
    • Kees Cook's avatar
      drm/mga/mga_ioc32: Use struct_group() for memcpy() region · 10579b75
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memcpy(), memmove(), and memset(), avoid
      intentionally writing across neighboring fields.
      
      Use struct_group() in struct drm32_mga_init around members chipset, sgram,
      maccess, fb_cpp, front_offset, front_pitch, back_offset, back_pitch,
      depth_cpp, depth_offset, depth_pitch, texture_offset, and texture_size,
      so they can be referenced together. This will allow memcpy() and sizeof()
      to more easily reason about sizes, improve readability, and avoid future
      warnings about writing beyond the end of chipset.
      
      "pahole" shows no size nor member offset changes to struct drm32_mga_init.
      "objdump -d" shows no meaningful object code changes (i.e. only source
      line number induced differences and optimizations).
      
      Note that since this is a UAPI header, __struct_group() is used
      directly.
      
      Cc: David Airlie <airlied@linux.ie>
      Cc: Lee Jones <lee.jones@linaro.org>
      Cc: dri-devel@lists.freedesktop.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarDaniel Vetter <daniel@ffwll.ch>
      Link: https://lore.kernel.org/lkml/YQKa76A6XuFqgM03@phenom.ffwll.local
      10579b75
    • Kees Cook's avatar
      iommu/amd: Use struct_group() for memcpy() region · 43d83af8
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memcpy(), memmove(), and memset(), avoid
      intentionally writing across neighboring fields.
      
      Use struct_group() in struct ivhd_entry around members ext and hidh, so
      they can be referenced together. This will allow memcpy() and sizeof()
      to more easily reason about sizes, improve readability, and avoid future
      warnings about writing beyond the end of ext.
      
      "pahole" shows no size nor member offset changes to struct ivhd_entry.
      "objdump -d" shows no object code changes.
      
      Cc: Will Deacon <will@kernel.org>
      Cc: iommu@lists.linux-foundation.org
      Acked-by: default avatarJoerg Roedel <jroedel@suse.de>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      43d83af8
    • Kees Cook's avatar
      bnxt_en: Use struct_group_attr() for memcpy() region · 241fe395
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memcpy(), memmove(), and memset(), avoid
      intentionally writing across neighboring fields.
      
      Use struct_group() around members queue_id, min_bw, max_bw, tsa, pri_lvl,
      and bw_weight so they can be referenced together. This will allow memcpy()
      and sizeof() to more easily reason about sizes, improve readability,
      and avoid future warnings about writing beyond the end of queue_id.
      
      "pahole" shows no size nor member offset changes to struct bnxt_cos2bw_cfg.
      "objdump -d" shows no meaningful object code changes (i.e. only source
      line number induced differences and optimizations).
      
      Cc: Michael Chan <michael.chan@broadcom.com>
      Cc: "David S. Miller" <davem@davemloft.net>
      Cc: Jakub Kicinski <kuba@kernel.org>
      Cc: netdev@vger.kernel.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Reviewed-by: default avatarMichael Chan <michael.chan@broadcom.com>
      Link: https://lore.kernel.org/lkml/CACKFLinDc6Y+P8eZ=450yA1nMC7swTURLtcdyiNR=9J6dfFyBg@mail.gmail.comReviewed-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
      Link: https://lore.kernel.org/lkml/20210728044517.GE35706@embeddedor
      241fe395
    • Kees Cook's avatar
      cxl/core: Replace unions with struct_group() · 301e68dd
      Kees Cook authored
      Use the newly introduced struct_group_typed() macro to clean up the
      declaration of struct cxl_regs.
      
      Cc: Alison Schofield <alison.schofield@intel.com>
      Cc: Vishal Verma <vishal.l.verma@intel.com>
      Cc: Ira Weiny <ira.weiny@intel.com>
      Cc: Ben Widawsky <ben.widawsky@intel.com>
      Cc: linux-cxl@vger.kernel.org
      Suggested-by: default avatarDan Williams <dan.j.williams@intel.com>
      Link: https://lore.kernel.org/lkml/1d9a2e6df2a9a35b2cdd50a9a68cac5991e7e5f0.camel@intel.comReviewed-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      301e68dd
    • Kees Cook's avatar
      stddef: Introduce struct_group() helper macro · 50d7bd38
      Kees Cook authored
      Kernel code has a regular need to describe groups of members within a
      structure usually when they need to be copied or initialized separately
      from the rest of the surrounding structure. The generally accepted design
      pattern in C is to use a named sub-struct:
      
      	struct foo {
      		int one;
      		struct {
      			int two;
      			int three, four;
      		} thing;
      		int five;
      	};
      
      This would allow for traditional references and sizing:
      
      	memcpy(&dst.thing, &src.thing, sizeof(dst.thing));
      
      However, doing this would mean that referencing struct members enclosed
      by such named structs would always require including the sub-struct name
      in identifiers:
      
      	do_something(dst.thing.three);
      
      This has tended to be quite inflexible, especially when such groupings
      need to be added to established code which causes huge naming churn.
      Three workarounds exist in the kernel for this problem, and each have
      other negative properties.
      
      To avoid the naming churn, there is a design pattern of adding macro
      aliases for the named struct:
      
      	#define f_three thing.three
      
      This ends up polluting the global namespace, and makes it difficult to
      search for identifiers.
      
      Another common work-around in kernel code avoids the pollution by avoiding
      the named struct entirely, instead identifying the group's boundaries using
      either a pair of empty anonymous structs of a pair of zero-element arrays:
      
      	struct foo {
      		int one;
      		struct { } start;
      		int two;
      		int three, four;
      		struct { } finish;
      		int five;
      	};
      
      	struct foo {
      		int one;
      		int start[0];
      		int two;
      		int three, four;
      		int finish[0];
      		int five;
      	};
      
      This allows code to avoid needing to use a sub-struct named for member
      references within the surrounding structure, but loses the benefits of
      being able to actually use such a struct, making it rather fragile. Using
      these requires open-coded calculation of sizes and offsets. The efforts
      made to avoid common mistakes include lots of comments, or adding various
      BUILD_BUG_ON()s. Such code is left with no way for the compiler to reason
      about the boundaries (e.g. the "start" object looks like it's 0 bytes
      in length), making bounds checking depend on open-coded calculations:
      
      	if (length > offsetof(struct foo, finish) -
      		     offsetof(struct foo, start))
      		return -EINVAL;
      	memcpy(&dst.start, &src.start, offsetof(struct foo, finish) -
      				       offsetof(struct foo, start));
      
      However, the vast majority of places in the kernel that operate on
      groups of members do so without any identification of the grouping,
      relying either on comments or implicit knowledge of the struct contents,
      which is even harder for the compiler to reason about, and results in
      even more fragile manual sizing, usually depending on member locations
      outside of the region (e.g. to copy "two" and "three", use the start of
      "four" to find the size):
      
      	BUILD_BUG_ON((offsetof(struct foo, four) <
      		      offsetof(struct foo, two)) ||
      		     (offsetof(struct foo, four) <
      		      offsetof(struct foo, three));
      	if (length > offsetof(struct foo, four) -
      		     offsetof(struct foo, two))
      		return -EINVAL;
      	memcpy(&dst.two, &src.two, length);
      
      In order to have a regular programmatic way to describe a struct
      region that can be used for references and sizing, can be examined for
      bounds checking, avoids forcing the use of intermediate identifiers,
      and avoids polluting the global namespace, introduce the struct_group()
      macro. This macro wraps the member declarations to create an anonymous
      union of an anonymous struct (no intermediate name) and a named struct
      (for references and sizing):
      
      	struct foo {
      		int one;
      		struct_group(thing,
      			int two;
      			int three, four;
      		);
      		int five;
      	};
      
      	if (length > sizeof(src.thing))
      		return -EINVAL;
      	memcpy(&dst.thing, &src.thing, length);
      	do_something(dst.three);
      
      There are some rare cases where the resulting struct_group() needs
      attributes added, so struct_group_attr() is also introduced to allow
      for specifying struct attributes (e.g. __align(x) or __packed).
      Additionally, there are places where such declarations would like to
      have the struct be tagged, so struct_group_tagged() is added.
      
      Given there is a need for a handful of UAPI uses too, the underlying
      __struct_group() macro has been defined in UAPI so it can be used there
      too.
      
      To avoid confusing scripts/kernel-doc, hide the macro from its struct
      parsing.
      Co-developed-by: default avatarKeith Packard <keithp@keithp.com>
      Signed-off-by: default avatarKeith Packard <keithp@keithp.com>
      Acked-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
      Link: https://lore.kernel.org/lkml/20210728023217.GC35706@embeddedorEnhanced-by: default avatarRasmus Villemoes <linux@rasmusvillemoes.dk>
      Link: https://lore.kernel.org/lkml/41183a98-bdb9-4ad6-7eab-5a7292a6df84@rasmusvillemoes.dkEnhanced-by: default avatarDan Williams <dan.j.williams@intel.com>
      Link: https://lore.kernel.org/lkml/1d9a2e6df2a9a35b2cdd50a9a68cac5991e7e5f0.camel@intel.comEnhanced-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
      Link: https://lore.kernel.org/lkml/YQKa76A6XuFqgM03@phenom.ffwll.localAcked-by: default avatarDan Williams <dan.j.williams@intel.com>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      50d7bd38
    • Kees Cook's avatar
      stddef: Fix kerndoc for sizeof_field() and offsetofend() · e7f18c22
      Kees Cook authored
      Adjust the comment styles so these are correctly identified as valid
      kern-doc.
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      e7f18c22
    • Kees Cook's avatar
      powerpc: Split memset() to avoid multi-field overflow · 0e17ad87
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Instead of writing across a field boundary with memset(), move the call
      to just the array, and an explicit zeroing of the prior field.
      
      Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
      Cc: Qinglang Miao <miaoqinglang@huawei.com>
      Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
      Cc: Hulk Robot <hulkci@huawei.com>
      Cc: Wang Wensheng <wangwensheng4@huawei.com>
      Cc: linuxppc-dev@lists.ozlabs.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Reviewed-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
      Link: https://lore.kernel.org/lkml/87czqsnmw9.fsf@mpe.ellerman.id.au
      0e17ad87
    • Kees Cook's avatar
      scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp · 3d0107a7
      Kees Cook authored
      In preparation for FORTIFY_SOURCE performing compile-time and run-time
      field bounds checking for memset(), avoid intentionally writing across
      neighboring fields.
      
      Instead of writing beyond the end of evt_struct->iu.srp.cmd, target the
      upper union (evt_struct->iu.srp) instead, as that's what is being wiped.
      
      Cc: Tyrel Datwyler <tyreld@linux.ibm.com>
      Cc: Michael Ellerman <mpe@ellerman.id.au>
      Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
      Cc: Paul Mackerras <paulus@samba.org>
      Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
      Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
      Cc: linux-scsi@vger.kernel.org
      Cc: linuxppc-dev@lists.ozlabs.org
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      Link: https://lore.kernel.org/lkml/yq135rzp79c.fsf@ca-mkp.ca.oracle.comAcked-by: default avatarTyrel Datwyler <tyreld@linux.ibm.com>
      Link: https://lore.kernel.org/lkml/6eae8434-e9a7-aa74-628b-b515b3695359@linux.ibm.com
      3d0107a7
  3. 20 Sep, 2021 2 commits
    • Linus Torvalds's avatar
      Linux 5.15-rc2 · e4e737bb
      Linus Torvalds authored
      e4e737bb
    • Linus Torvalds's avatar
      pci_iounmap'2: Electric Boogaloo: try to make sense of it all · 316e8d79
      Linus Torvalds authored
      Nathan Chancellor reports that the recent change to pci_iounmap in
      commit 9caea000 ("parisc: Declare pci_iounmap() parisc version only
      when CONFIG_PCI enabled") causes build errors on arm64.
      
      It took me about two hours to convince myself that I think I know what
      the logic of that mess of #ifdef's in the <asm-generic/io.h> header file
      really aim to do, and rewrite it to be easier to follow.
      
      Famous last words.
      
      Anyway, the code has now been lifted from that grotty header file into
      lib/pci_iomap.c, and has fairly extensive comments about what the logic
      is.  It also avoids indirecting through another confusing (and badly
      named) helper function that has other preprocessor config conditionals.
      
      Let's see what odd architecture did something else strange in this area
      to break things.  But my arm64 cross build is clean.
      
      Fixes: 9caea000 ("parisc: Declare pci_iounmap() parisc version only when CONFIG_PCI enabled")
      Reported-by: default avatarNathan Chancellor <nathan@kernel.org>
      Cc: Helge Deller <deller@gmx.de>
      Cc: Arnd Bergmann <arnd@arndb.de>
      Cc: Guenter Roeck <linux@roeck-us.net>
      Cc: Ulrich Teichert <krypton@ulrich-teichert.org>
      Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      316e8d79
  4. 19 Sep, 2021 12 commits