From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Kees Cook <keescook@chromium.org>
Cc: linux-hardening@vger.kernel.org,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Keith Packard <keithpac@amazon.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
linux-staging@lists.linux.dev, linux-block@vger.kernel.org,
linux-kbuild@vger.kernel.org, clang-built-linux@googlegroups.com
Subject: Re: [PATCH 25/64] drm/mga/mga_ioc32: Use struct_group() for memcpy() region
Date: Wed, 28 Jul 2021 07:56:40 +0200 [thread overview]
Message-ID: <YQDxmEYfppJ4wAmD@kroah.com> (raw)
In-Reply-To: <20210727205855.411487-26-keescook@chromium.org>
On Tue, Jul 27, 2021 at 01:58:16PM -0700, Kees Cook wrote:
> 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 includes a UAPI header, struct_group() has been
> explicitly redefined local to the header.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/gpu/drm/mga/mga_ioc32.c | 30 ++++++++++++++------------
> include/uapi/drm/mga_drm.h | 37 ++++++++++++++++++++++++---------
> 2 files changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/mga/mga_ioc32.c b/drivers/gpu/drm/mga/mga_ioc32.c
> index 4fd4de16cd32..fbd0329dbd4f 100644
> --- a/drivers/gpu/drm/mga/mga_ioc32.c
> +++ b/drivers/gpu/drm/mga/mga_ioc32.c
> @@ -38,16 +38,21 @@
> typedef struct drm32_mga_init {
> int func;
> u32 sarea_priv_offset;
> - int chipset;
> - int sgram;
> - unsigned int maccess;
> - unsigned int fb_cpp;
> - unsigned int front_offset, front_pitch;
> - unsigned int back_offset, back_pitch;
> - unsigned int depth_cpp;
> - unsigned int depth_offset, depth_pitch;
> - unsigned int texture_offset[MGA_NR_TEX_HEAPS];
> - unsigned int texture_size[MGA_NR_TEX_HEAPS];
> + struct_group(always32bit,
> + int chipset;
> + int sgram;
> + unsigned int maccess;
> + unsigned int fb_cpp;
> + unsigned int front_offset;
> + unsigned int front_pitch;
> + unsigned int back_offset;
> + unsigned int back_pitch;
> + unsigned int depth_cpp;
> + unsigned int depth_offset;
> + unsigned int depth_pitch;
> + unsigned int texture_offset[MGA_NR_TEX_HEAPS];
> + unsigned int texture_size[MGA_NR_TEX_HEAPS];
> + );
> u32 fb_offset;
> u32 mmio_offset;
> u32 status_offset;
> @@ -67,9 +72,8 @@ static int compat_mga_init(struct file *file, unsigned int cmd,
>
> init.func = init32.func;
> init.sarea_priv_offset = init32.sarea_priv_offset;
> - memcpy(&init.chipset, &init32.chipset,
> - offsetof(drm_mga_init_t, fb_offset) -
> - offsetof(drm_mga_init_t, chipset));
> + memcpy(&init.always32bit, &init32.always32bit,
> + sizeof(init32.always32bit));
> init.fb_offset = init32.fb_offset;
> init.mmio_offset = init32.mmio_offset;
> init.status_offset = init32.status_offset;
> diff --git a/include/uapi/drm/mga_drm.h b/include/uapi/drm/mga_drm.h
> index 8c4337548ab5..61612e5ecab2 100644
> --- a/include/uapi/drm/mga_drm.h
> +++ b/include/uapi/drm/mga_drm.h
> @@ -265,6 +265,16 @@ typedef struct _drm_mga_sarea {
> #define DRM_IOCTL_MGA_WAIT_FENCE DRM_IOWR(DRM_COMMAND_BASE + DRM_MGA_WAIT_FENCE, __u32)
> #define DRM_IOCTL_MGA_DMA_BOOTSTRAP DRM_IOWR(DRM_COMMAND_BASE + DRM_MGA_DMA_BOOTSTRAP, drm_mga_dma_bootstrap_t)
>
> +#define __struct_group(name, fields) \
> + union { \
> + struct { \
> + fields \
> + }; \
> + struct { \
> + fields \
> + } name; \
> + }
> +
> typedef struct _drm_mga_warp_index {
> int installed;
> unsigned long phys_addr;
> @@ -279,20 +289,25 @@ typedef struct drm_mga_init {
>
> unsigned long sarea_priv_offset;
>
> - int chipset;
> - int sgram;
> + __struct_group(always32bit,
> + int chipset;
> + int sgram;
>
> - unsigned int maccess;
> + unsigned int maccess;
>
> - unsigned int fb_cpp;
> - unsigned int front_offset, front_pitch;
> - unsigned int back_offset, back_pitch;
> + unsigned int fb_cpp;
> + unsigned int front_offset;
> + unsigned int front_pitch;
> + unsigned int back_offset;
> + unsigned int back_pitch;
>
> - unsigned int depth_cpp;
> - unsigned int depth_offset, depth_pitch;
> + unsigned int depth_cpp;
> + unsigned int depth_offset;
> + unsigned int depth_pitch;
>
> - unsigned int texture_offset[MGA_NR_TEX_HEAPS];
> - unsigned int texture_size[MGA_NR_TEX_HEAPS];
> + unsigned int texture_offset[MGA_NR_TEX_HEAPS];
> + unsigned int texture_size[MGA_NR_TEX_HEAPS];
> + );
>
> unsigned long fb_offset;
> unsigned long mmio_offset;
> @@ -302,6 +317,8 @@ typedef struct drm_mga_init {
> unsigned long buffers_offset;
> } drm_mga_init_t;
>
> +#undef __struct_group
> +
Why can you use __struct_group in this uapi header, but not the
networking one?
thanks,
greg k-h
next prev parent reply other threads:[~2021-07-28 5:56 UTC|newest]
Thread overview: 158+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-27 20:57 [PATCH 00/64] Introduce strict memcpy() bounds checking Kees Cook
2021-07-27 20:57 ` [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region Kees Cook
2021-07-28 0:55 ` Gustavo A. R. Silva
2021-07-28 1:50 ` Kees Cook
2021-07-28 8:59 ` David Sterba
2021-07-28 9:14 ` Dan Carpenter
2021-07-28 21:37 ` Bart Van Assche
2021-07-28 21:37 ` David Sterba
2021-07-29 5:56 ` Greg Kroah-Hartman
2021-07-29 8:20 ` Dan Carpenter
2021-07-30 6:00 ` Kees Cook
2021-07-30 8:38 ` David Sterba
2021-07-30 9:00 ` Dan Carpenter
2021-07-30 16:44 ` Kees Cook
2021-07-30 17:08 ` Nick Desaulniers
2021-07-30 19:18 ` Kees Cook
2021-07-27 20:57 ` [PATCH 02/64] mac80211: Use flex-array for radiotap header bitmap Kees Cook
2021-07-28 7:35 ` Dan Carpenter
2021-07-28 9:23 ` David Sterba
2021-07-28 21:54 ` Kees Cook
2021-07-29 10:45 ` David Sterba
2021-07-30 6:06 ` Kees Cook
2021-07-28 21:20 ` Kees Cook
2021-07-28 23:14 ` Kees Cook
2021-07-28 23:33 ` Kees Cook
2021-07-29 8:25 ` Dan Carpenter
2021-07-27 20:57 ` [PATCH 03/64] rpmsg: glink: Replace strncpy() with strscpy_pad() Kees Cook
2021-07-28 2:07 ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 04/64] stddef: Introduce struct_group() helper macro Kees Cook
2021-07-28 2:32 ` Gustavo A. R. Silva
2021-07-28 10:54 ` Rasmus Villemoes
2021-07-28 21:59 ` Kees Cook
2021-07-30 22:19 ` Williams, Dan J
2021-07-31 2:59 ` Kees Cook
[not found] ` <CAKwiHFheDv2pwsm6Fa+-KnOFyvk7bfZQjb2BQ-CSwH61gxgVYg@mail.gmail.com>
2021-07-31 15:10 ` Kees Cook
2021-07-27 20:57 ` [PATCH 05/64] skbuff: Switch structure bounds to struct_group() Kees Cook
2021-07-28 3:50 ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 06/64] bnxt_en: Use struct_group_attr() for memcpy() region Kees Cook
2021-07-28 1:03 ` Michael Chan
2021-07-28 4:45 ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 07/64] staging: rtl8192e: Use struct_group() " Kees Cook
2021-07-27 22:30 ` Larry Finger
2021-07-28 5:45 ` Greg Kroah-Hartman
2021-07-27 20:57 ` [PATCH 08/64] staging: rtl8192u: " Kees Cook
2021-07-28 5:45 ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 09/64] staging: rtl8723bs: Avoid field-overflowing memcpy() Kees Cook
2021-07-28 5:46 ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 10/64] lib80211: Use struct_group() for memcpy() region Kees Cook
2021-07-28 5:52 ` Greg Kroah-Hartman
2021-08-13 8:04 ` Johannes Berg
2021-08-13 15:49 ` Kees Cook
2021-08-13 19:44 ` Johannes Berg
2021-07-27 20:58 ` [PATCH 11/64] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
2021-07-27 20:58 ` [PATCH 12/64] mwl8k: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58 ` [PATCH 13/64] libertas: " Kees Cook
2021-07-27 20:58 ` [PATCH 14/64] libertas_tf: " Kees Cook
2021-07-27 20:58 ` [PATCH 15/64] ipw2x00: " Kees Cook
2021-07-28 18:55 ` Stanislav Yakovlev
2021-07-27 20:58 ` [PATCH 16/64] thermal: intel: int340x_thermal: " Kees Cook
2021-07-27 20:58 ` [PATCH 17/64] iommu/amd: " Kees Cook
2021-07-27 20:58 ` [PATCH 18/64] cxgb3: " Kees Cook
2021-07-27 20:58 ` [PATCH 19/64] ip: Use struct_group() for memcpy() regions Kees Cook
2021-07-28 5:55 ` Greg Kroah-Hartman
2021-07-28 6:14 ` Gustavo A. R. Silva
2021-07-28 6:19 ` Greg Kroah-Hartman
2021-07-28 6:31 ` Gustavo A. R. Silva
2021-07-28 6:37 ` Gustavo A. R. Silva
2021-07-28 6:41 ` Greg Kroah-Hartman
2021-07-28 21:01 ` Kees Cook
2021-07-29 1:59 ` Bart Van Assche
2021-07-27 20:58 ` [PATCH 20/64] intersil: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58 ` [PATCH 21/64] cxgb4: " Kees Cook
2021-07-27 20:58 ` [PATCH 22/64] bnx2x: " Kees Cook
2021-07-27 20:58 ` [PATCH 23/64] drm/amd/pm: " Kees Cook
2021-07-30 2:07 ` Alex Deucher
2021-07-27 20:58 ` [PATCH 24/64] staging: wlan-ng: " Kees Cook
2021-07-28 5:45 ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 25/64] drm/mga/mga_ioc32: " Kees Cook
2021-07-28 5:56 ` Greg Kroah-Hartman [this message]
2021-07-29 12:11 ` Daniel Vetter
2021-07-31 4:20 ` Kees Cook
2021-07-27 20:58 ` [PATCH 26/64] net/mlx5e: " Kees Cook
2021-07-27 20:58 ` [PATCH 27/64] HID: cp2112: " Kees Cook
2021-07-27 20:58 ` [PATCH 28/64] compiler_types.h: Remove __compiletime_object_size() Kees Cook
2021-07-27 20:58 ` [PATCH 29/64] lib/string: Move helper functions out of string.c Kees Cook
2021-07-27 20:58 ` [PATCH 30/64] fortify: Move remaining fortify helpers into fortify-string.h Kees Cook
2021-07-27 20:58 ` [PATCH 31/64] fortify: Explicitly disable Clang support Kees Cook
2021-07-27 21:18 ` Nathan Chancellor
2021-07-27 21:47 ` Kees Cook
2021-07-27 20:58 ` [PATCH 32/64] fortify: Add compile-time FORTIFY_SOURCE tests Kees Cook
2021-07-27 20:58 ` [PATCH 33/64] lib: Introduce CONFIG_TEST_MEMCPY Kees Cook
2021-07-27 23:31 ` Bart Van Assche
2021-07-27 23:33 ` Randy Dunlap
2021-07-28 1:30 ` Kees Cook
2021-07-27 20:58 ` [PATCH 34/64] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
2021-07-27 22:43 ` Nick Desaulniers
2021-07-28 1:47 ` Kees Cook
2021-07-28 11:19 ` Rasmus Villemoes
2021-07-30 2:39 ` Kees Cook
2021-07-27 20:58 ` [PATCH 35/64] fortify: Detect struct member overflows in memmove() " Kees Cook
2021-07-27 20:58 ` [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp Kees Cook
2021-07-28 1:39 ` Martin K. Petersen
2021-07-28 18:57 ` Kees Cook
2021-07-29 3:35 ` Martin K. Petersen
2021-07-30 19:11 ` Tyrel Datwyler
2021-07-30 18:16 ` Tyrel Datwyler
2021-07-27 20:58 ` [PATCH 37/64] string.h: Introduce memset_after() for wiping trailing members/padding Kees Cook
2021-07-27 20:58 ` [PATCH 38/64] xfrm: Use memset_after() to clear padding Kees Cook
2021-07-27 20:58 ` [PATCH 39/64] mac80211: Use memset_after() to clear tx status Kees Cook
2021-07-31 15:55 ` Kees Cook
2021-08-13 7:40 ` Johannes Berg
2021-08-13 16:08 ` Kees Cook
2021-08-13 18:19 ` Johannes Berg
2021-08-13 7:41 ` Johannes Berg
2021-07-27 20:58 ` [PATCH 40/64] net: 802: Use memset_after() to clear struct fields Kees Cook
2021-07-27 20:58 ` [PATCH 41/64] net: dccp: Use memset_after() for TP zeroing Kees Cook
2021-07-27 20:58 ` [PATCH 42/64] net: qede: Use memset_after() for counters Kees Cook
2021-07-31 16:07 ` Kees Cook
2021-07-27 20:58 ` [PATCH 43/64] ath11k: Use memset_after() for clearing queue descriptors Kees Cook
2021-07-27 20:58 ` [PATCH 44/64] iw_cxgb4: Use memset_after() for cpl_t5_pass_accept_rpl Kees Cook
2021-07-27 20:58 ` [PATCH 45/64] intel_th: msu: Use memset_after() for clearing hw header Kees Cook
2021-07-27 20:58 ` [PATCH 46/64] IB/mthca: Use memset_after() for clearing mpt_entry Kees Cook
2021-07-27 20:58 ` [PATCH 47/64] btrfs: Use memset_after() to clear end of struct Kees Cook
2021-07-28 9:42 ` David Sterba
2021-07-28 21:56 ` Kees Cook
2021-07-29 10:33 ` David Sterba
2021-07-31 15:25 ` Kees Cook
2021-08-09 11:20 ` David Sterba
2021-07-27 20:58 ` [PATCH 48/64] drbd: Use struct_group() to zero algs Kees Cook
2021-07-28 21:45 ` Bart Van Assche
2021-07-30 2:31 ` Kees Cook
2021-07-30 2:57 ` Bart Van Assche
2021-07-30 9:25 ` Lars Ellenberg
2021-07-30 15:32 ` Nick Desaulniers
2021-07-27 20:58 ` [PATCH 49/64] cm4000_cs: Use struct_group() to zero struct cm4000_dev region Kees Cook
2021-07-28 5:48 ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 50/64] KVM: x86: Use struct_group() to zero decode cache Kees Cook
2021-07-27 20:58 ` [PATCH 51/64] tracing: Use struct_group() to zero struct trace_iterator Kees Cook
2021-07-27 20:58 ` [PATCH 52/64] dm integrity: Use struct_group() to zero struct journal_sector Kees Cook
2021-07-27 20:58 ` [PATCH 53/64] HID: roccat: Use struct_group() to zero kone_mouse_event Kees Cook
2021-07-27 20:58 ` [PATCH 54/64] ipv6: Use struct_group() to zero rt6_info Kees Cook
2021-07-29 18:58 ` Jakub Kicinski
2021-07-31 15:01 ` Kees Cook
2021-07-27 20:58 ` [PATCH 55/64] RDMA/mlx5: Use struct_group() to zero struct mlx5_ib_mr Kees Cook
2021-07-27 20:58 ` [PATCH 56/64] ethtool: stats: Use struct_group() to clear all stats at once Kees Cook
2021-07-27 20:58 ` [PATCH 57/64] netfilter: conntrack: Use struct_group() to zero struct nf_conn Kees Cook
2021-07-27 20:58 ` [PATCH 58/64] powerpc: Split memset() to avoid multi-field overflow Kees Cook
2021-08-05 11:36 ` Michael Ellerman
2021-07-27 20:58 ` [PATCH 59/64] fortify: Detect struct member overflows in memset() at compile-time Kees Cook
2021-07-27 20:58 ` [PATCH 60/64] fortify: Work around Clang inlining bugs Kees Cook
2021-07-27 20:58 ` [PATCH 61/64] Makefile: Enable -Warray-bounds Kees Cook
2021-07-27 20:58 ` [PATCH 62/64] netlink: Avoid false-positive memcpy() warning Kees Cook
2021-07-28 5:49 ` Greg Kroah-Hartman
2021-07-28 11:24 ` Rasmus Villemoes
2021-07-30 1:39 ` Kees Cook
2021-07-30 1:41 ` Kees Cook
2021-07-27 20:58 ` [PATCH 63/64] iwlwifi: dbg_ini: Split memcpy() to avoid multi-field write Kees Cook
2021-07-27 20:58 ` [PATCH 64/64] fortify: Add run-time WARN for cross-field memcpy() Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YQDxmEYfppJ4wAmD@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=clang-built-linux@googlegroups.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavoars@kernel.org \
--cc=keescook@chromium.org \
--cc=keithpac@amazon.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).