| Message ID | 20210818060533.3569517-1-keescook@chromium.org |
|---|---|
| Headers | show |
| Series | Introduce strict memcpy() bounds checking | expand |
On Tue, 17 Aug 2021 23:05:20 -0700 Kees Cook <keescook@chromium.org> wrote: > 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() to avoid confusing memset() about writing beyond > the target struct member. > > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Ingo Molnar <mingo@redhat.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > kernel/trace/trace.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 13587e771567..9ff8c31975cd 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -6691,9 +6691,7 @@ tracing_read_pipe(struct file *filp, char __user *ubuf, > cnt = PAGE_SIZE - 1; > > /* reset all but tr, trace, and overruns */ > - memset(&iter->seq, 0, > - sizeof(struct trace_iterator) - > - offsetof(struct trace_iterator, seq)); > + memset_startat(iter, 0, seq); I can't find memset_startat() in mainline nor linux-next. I don't see it in this thread either, but since this has 63 patches, I could have easily missed it. This change really should belong to a patch set that just introduces memset_startat() (and perhaps memset_after()) and then updates all the places that should use it. That way I can give it a proper review. In other words, you should break this patch set up into smaller, more digestible portions for the reviewers. Thanks, -- Steve > cpumask_clear(iter->started); > trace_seq_init(&iter->seq); > iter->pos = -1;
On Wed, Aug 18, 2021 at 09:33:49AM -0400, Steven Rostedt wrote: > On Tue, 17 Aug 2021 23:05:20 -0700 > Kees Cook <keescook@chromium.org> wrote: > > > 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() to avoid confusing memset() about writing beyond > > the target struct member. > > > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Ingo Molnar <mingo@redhat.com> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > kernel/trace/trace.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index 13587e771567..9ff8c31975cd 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -6691,9 +6691,7 @@ tracing_read_pipe(struct file *filp, char __user *ubuf, > > cnt = PAGE_SIZE - 1; > > > > /* reset all but tr, trace, and overruns */ > > - memset(&iter->seq, 0, > > - sizeof(struct trace_iterator) - > > - offsetof(struct trace_iterator, seq)); > > + memset_startat(iter, 0, seq); > > I can't find memset_startat() in mainline nor linux-next. I don't see it > in this thread either, but since this has 63 patches, I could have > easily missed it. Sorry, it isn't called out in the Subject, but it's part of this patch: https://lore.kernel.org/lkml/20210818060533.3569517-38-keescook@chromium.org/ > This change really should belong to a patch set that just introduces > memset_startat() (and perhaps memset_after()) and then updates all the > places that should use it. That way I can give it a proper review. In > other words, you should break this patch set up into smaller, more > digestible portions for the reviewers. I will split memset_after() and memset_startat() introduction patches. I already split up each use into individual cases, so that those changes could be checked one step at a time for differences in pahole struct layout and object code. Thanks for taking a look! -Kees
On Tue, Aug 17, 2021 at 11:06 PM Kees Cook <keescook@chromium.org> wrote: > > 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: Dan Williams <dan.j.williams@intel.com> Looks good and tests ok here: Reviewed-by: Dan Williams <dan.j.williams@intel.com>
On Wed, Aug 18, 2021 at 10:53:58PM +0000, Sean Christopherson wrote: > On Wed, Aug 18, 2021, Kees Cook wrote: > > On Wed, Aug 18, 2021 at 03:11:28PM +0000, Sean Christopherson wrote: > > > From dbdca1f4cd01fee418c252e54c360d518b2b1ad6 Mon Sep 17 00:00:00 2001 > > > From: Sean Christopherson <seanjc@google.com> > > > Date: Wed, 18 Aug 2021 08:03:08 -0700 > > > Subject: [PATCH] KVM: x86: Replace memset() "optimization" with normal > > > per-field writes > > > > > > Explicitly zero select fields in the emulator's decode cache instead of > > > zeroing the fields via a gross memset() that spans six fields. gcc and > > > clang are both clever enough to batch the first five fields into a single > > > quadword MOV, i.e. memset() and individually zeroing generate identical > > > code. > > > > > > Removing the wart also prepares KVM for FORTIFY_SOURCE performing > > > compile-time and run-time field bounds checking for memset(). > > > > > > No functional change intended. > > > > > > Reported-by: Kees Cook <keescook@chromium.org> > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > > > Do you want me to take this patch into my tree, or do you want to carry > > it for KVM directly? > > That's a Paolo question :-) > > What's the expected timeframe for landing stricter bounds checking? If it's > 5.16 or later, the easiest thing would be to squeak this into 5.15. I'm hoping to land all the "compile time" stuff for 5.15, but realistically, some portions may not get there. I'll just carry this patch for now and if we need to swap trees we can do that. :) Thanks! -Kees
On Tue, Aug 17, 2021 at 11:05:26PM -0700, Kees Cook wrote: > 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 mlx5_ib_mr that should be > initialized to zero. > > Cc: Leon Romanovsky <leon@kernel.org> > Cc: Doug Ledford <dledford@redhat.com> > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Cc: linux-rdma@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h > index bf20a388eabe..f63bf204a7a1 100644 > --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h > @@ -644,6 +644,7 @@ struct mlx5_ib_mr { > struct ib_umem *umem; > > /* This is zero'd when the MR is allocated */ > + struct_group(cleared, > union { > /* Used only while the MR is in the cache */ > struct { > @@ -691,12 +692,13 @@ struct mlx5_ib_mr { > bool is_odp_implicit; > }; > }; > + ); > }; > > /* Zero the fields in the mr that are variant depending on usage */ > static inline void mlx5_clear_mr(struct mlx5_ib_mr *mr) > { > - memset(mr->out, 0, sizeof(*mr) - offsetof(struct mlx5_ib_mr, out)); > + memset(&mr->cleared, 0, sizeof(mr->cleared)); > } Why not use the memset_after(mr->umem) here? Jason
On Thu, Aug 19, 2021 at 09:27:16AM -0300, Jason Gunthorpe wrote: > On Tue, Aug 17, 2021 at 11:05:26PM -0700, Kees Cook wrote: > > 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 mlx5_ib_mr that should be > > initialized to zero. > > > > Cc: Leon Romanovsky <leon@kernel.org> > > Cc: Doug Ledford <dledford@redhat.com> > > Cc: Jason Gunthorpe <jgg@ziepe.ca> > > Cc: linux-rdma@vger.kernel.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h > > index bf20a388eabe..f63bf204a7a1 100644 > > --- a/drivers/infiniband/hw/mlx5/mlx5_ib.h > > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h > > @@ -644,6 +644,7 @@ struct mlx5_ib_mr { > > struct ib_umem *umem; > > > > /* This is zero'd when the MR is allocated */ > > + struct_group(cleared, > > union { > > /* Used only while the MR is in the cache */ > > struct { > > @@ -691,12 +692,13 @@ struct mlx5_ib_mr { > > bool is_odp_implicit; > > }; > > }; > > + ); > > }; > > > > /* Zero the fields in the mr that are variant depending on usage */ > > static inline void mlx5_clear_mr(struct mlx5_ib_mr *mr) > > { > > - memset(mr->out, 0, sizeof(*mr) - offsetof(struct mlx5_ib_mr, out)); > > + memset(&mr->cleared, 0, sizeof(mr->cleared)); > > } > > Why not use the memset_after(mr->umem) here? I can certainly do that instead. In this series I've tended to opt for groupings so the position of future struct member additions are explicitly chosen. (i.e. reducing the chance that a zeroing of the new member be a surprise.) -Kees -- Kees Cook
On Thu, Aug 19, 2021 at 09:19:08AM -0700, Kees Cook wrote: > On Thu, Aug 19, 2021 at 09:27:16AM -0300, Jason Gunthorpe wrote: > > On Tue, Aug 17, 2021 at 11:05:26PM -0700, Kees Cook wrote: > > > 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 mlx5_ib_mr that should be > > > initialized to zero. > > > > > > Cc: Leon Romanovsky <leon@kernel.org> > > > Cc: Doug Ledford <dledford@redhat.com> > > > Cc: Jason Gunthorpe <jgg@ziepe.ca> > > > Cc: linux-rdma@vger.kernel.org > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h > > > index bf20a388eabe..f63bf204a7a1 100644 > > > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h > > > @@ -644,6 +644,7 @@ struct mlx5_ib_mr { > > > struct ib_umem *umem; > > > > > > /* This is zero'd when the MR is allocated */ > > > + struct_group(cleared, > > > union { > > > /* Used only while the MR is in the cache */ > > > struct { > > > @@ -691,12 +692,13 @@ struct mlx5_ib_mr { > > > bool is_odp_implicit; > > > }; > > > }; > > > + ); > > > }; > > > > > > /* Zero the fields in the mr that are variant depending on usage */ > > > static inline void mlx5_clear_mr(struct mlx5_ib_mr *mr) > > > { > > > - memset(mr->out, 0, sizeof(*mr) - offsetof(struct mlx5_ib_mr, out)); > > > + memset(&mr->cleared, 0, sizeof(mr->cleared)); > > > } > > > > Why not use the memset_after(mr->umem) here? > > I can certainly do that instead. In this series I've tended to opt > for groupings so the position of future struct member additions are > explicitly chosen. (i.e. reducing the chance that a zeroing of the new > member be a surprise.) I saw the earlier RDMA patches where using other memset techniques though? Were there flex arrays or something that made groups infeasible? Jason
On Thu, Aug 19, 2021 at 01:47:57PM -0300, Jason Gunthorpe wrote: > On Thu, Aug 19, 2021 at 09:19:08AM -0700, Kees Cook wrote: > > On Thu, Aug 19, 2021 at 09:27:16AM -0300, Jason Gunthorpe wrote: > > > On Tue, Aug 17, 2021 at 11:05:26PM -0700, Kees Cook wrote: > > > > 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 mlx5_ib_mr that should be > > > > initialized to zero. > > > > > > > > Cc: Leon Romanovsky <leon@kernel.org> > > > > Cc: Doug Ledford <dledford@redhat.com> > > > > Cc: Jason Gunthorpe <jgg@ziepe.ca> > > > > Cc: linux-rdma@vger.kernel.org > > > > Signed-off-by: Kees Cook <keescook@chromium.org> > > > > drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h > > > > index bf20a388eabe..f63bf204a7a1 100644 > > > > +++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h > > > > @@ -644,6 +644,7 @@ struct mlx5_ib_mr { > > > > struct ib_umem *umem; > > > > > > > > /* This is zero'd when the MR is allocated */ > > > > + struct_group(cleared, > > > > union { > > > > /* Used only while the MR is in the cache */ > > > > struct { > > > > @@ -691,12 +692,13 @@ struct mlx5_ib_mr { > > > > bool is_odp_implicit; > > > > }; > > > > }; > > > > + ); > > > > }; > > > > > > > > /* Zero the fields in the mr that are variant depending on usage */ > > > > static inline void mlx5_clear_mr(struct mlx5_ib_mr *mr) > > > > { > > > > - memset(mr->out, 0, sizeof(*mr) - offsetof(struct mlx5_ib_mr, out)); > > > > + memset(&mr->cleared, 0, sizeof(mr->cleared)); > > > > } > > > > > > Why not use the memset_after(mr->umem) here? > > > > I can certainly do that instead. In this series I've tended to opt > > for groupings so the position of future struct member additions are > > explicitly chosen. (i.e. reducing the chance that a zeroing of the new > > member be a surprise.) > > I saw the earlier RDMA patches where using other memset techniques > though? Were there flex arrays or something that made groups infeasible? Which do you mean? When doing the conversions I tended to opt for struct_group() since it provides more robust "intentionality". Strictly speaking, the new memset helpers are doing field-spanning writes, but the "clear to the end" pattern was so common it made sense to add the helpers, as they're a bit less disruptive. It's totally up to you! :)
Kees Cook <keescook@chromium.org> writes: > In preparation for FORTIFY_SOURCE performing compile-time and run-time > field bounds checking for memset(), avoid intentionally writing across > neighboring fields. > > Add a struct_group() for the spe registers so that memset() can correctly reason > about the size: > > In function 'fortify_memset_chk', > inlined from 'restore_user_regs.part.0' at arch/powerpc/kernel/signal_32.c:539:3: >>> include/linux/fortify-string.h:195:4: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] > 195 | __write_overflow_field(); > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > Cc: Sudeep Holla <sudeep.holla@arm.com> > Cc: linuxppc-dev@lists.ozlabs.org > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > arch/powerpc/include/asm/processor.h | 6 ++++-- > arch/powerpc/kernel/signal_32.c | 6 +++--- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h > index f348e564f7dd..05dc567cb9a8 100644 > --- a/arch/powerpc/include/asm/processor.h > +++ b/arch/powerpc/include/asm/processor.h > @@ -191,8 +191,10 @@ struct thread_struct { > int used_vsr; /* set if process has used VSX */ > #endif /* CONFIG_VSX */ > #ifdef CONFIG_SPE > - unsigned long evr[32]; /* upper 32-bits of SPE regs */ > - u64 acc; /* Accumulator */ > + struct_group(spe, > + unsigned long evr[32]; /* upper 32-bits of SPE regs */ > + u64 acc; /* Accumulator */ > + ); > unsigned long spefscr; /* SPE & eFP status */ > unsigned long spefscr_last; /* SPEFSCR value on last prctl > call or trap return */ > diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c > index 0608581967f0..77b86caf5c51 100644 > --- a/arch/powerpc/kernel/signal_32.c > +++ b/arch/powerpc/kernel/signal_32.c > @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs, > regs_set_return_msr(regs, regs->msr & ~MSR_SPE); > if (msr & MSR_SPE) { > /* restore spe registers from the stack */ > - unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs, > - ELF_NEVRREG * sizeof(u32), failed); > + unsafe_copy_from_user(¤t->thread.spe, &sr->mc_vregs, > + sizeof(current->thread.spe), failed); This makes me nervous, because the ABI is that we copy ELF_NEVRREG * sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to be. ie. if we use sizeof an inadvertent change to the fields in thread_struct could change how many bytes we copy out to userspace, which would be an ABI break. And that's not that hard to do, because it's not at all obvious that the size and layout of fields in thread_struct affects the user ABI. At the same time we don't want to copy the right number of bytes but the wrong content, so from that point of view using sizeof is good :) The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that things match up, so maybe we should do that here too. ie. add: BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32)); Not sure if you are happy doing that as part of this patch. I can always do it later if not. cheers
Le 20/08/2021 à 09:49, Michael Ellerman a écrit : > Kees Cook <keescook@chromium.org> writes: >> In preparation for FORTIFY_SOURCE performing compile-time and run-time >> field bounds checking for memset(), avoid intentionally writing across >> neighboring fields. >> >> Add a struct_group() for the spe registers so that memset() can correctly reason >> about the size: >> >> In function 'fortify_memset_chk', >> inlined from 'restore_user_regs.part.0' at arch/powerpc/kernel/signal_32.c:539:3: >>>> include/linux/fortify-string.h:195:4: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] >> 195 | __write_overflow_field(); >> | ^~~~~~~~~~~~~~~~~~~~~~~~ >> >> Cc: Michael Ellerman <mpe@ellerman.id.au> >> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Christophe Leroy <christophe.leroy@csgroup.eu> >> Cc: Sudeep Holla <sudeep.holla@arm.com> >> Cc: linuxppc-dev@lists.ozlabs.org >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> arch/powerpc/include/asm/processor.h | 6 ++++-- >> arch/powerpc/kernel/signal_32.c | 6 +++--- >> 2 files changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h >> index f348e564f7dd..05dc567cb9a8 100644 >> --- a/arch/powerpc/include/asm/processor.h >> +++ b/arch/powerpc/include/asm/processor.h >> @@ -191,8 +191,10 @@ struct thread_struct { >> int used_vsr; /* set if process has used VSX */ >> #endif /* CONFIG_VSX */ >> #ifdef CONFIG_SPE >> - unsigned long evr[32]; /* upper 32-bits of SPE regs */ >> - u64 acc; /* Accumulator */ >> + struct_group(spe, >> + unsigned long evr[32]; /* upper 32-bits of SPE regs */ >> + u64 acc; /* Accumulator */ >> + ); >> unsigned long spefscr; /* SPE & eFP status */ >> unsigned long spefscr_last; /* SPEFSCR value on last prctl >> call or trap return */ >> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c >> index 0608581967f0..77b86caf5c51 100644 >> --- a/arch/powerpc/kernel/signal_32.c >> +++ b/arch/powerpc/kernel/signal_32.c >> @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs, >> regs_set_return_msr(regs, regs->msr & ~MSR_SPE); >> if (msr & MSR_SPE) { >> /* restore spe registers from the stack */ >> - unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs, >> - ELF_NEVRREG * sizeof(u32), failed); >> + unsafe_copy_from_user(¤t->thread.spe, &sr->mc_vregs, >> + sizeof(current->thread.spe), failed); > > This makes me nervous, because the ABI is that we copy ELF_NEVRREG * > sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to > be. > > ie. if we use sizeof an inadvertent change to the fields in > thread_struct could change how many bytes we copy out to userspace, > which would be an ABI break. > > And that's not that hard to do, because it's not at all obvious that the > size and layout of fields in thread_struct affects the user ABI. > > At the same time we don't want to copy the right number of bytes but > the wrong content, so from that point of view using sizeof is good :) > > The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that > things match up, so maybe we should do that here too. > > ie. add: > > BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32)); You mean != I guess ? > > > Not sure if you are happy doing that as part of this patch. I can always > do it later if not. > > cheers >
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 20/08/2021 à 09:49, Michael Ellerman a écrit : >> Kees Cook <keescook@chromium.org> writes: >>> In preparation for FORTIFY_SOURCE performing compile-time and run-time >>> field bounds checking for memset(), avoid intentionally writing across >>> neighboring fields. >>> >>> Add a struct_group() for the spe registers so that memset() can correctly reason >>> about the size: >>> >>> In function 'fortify_memset_chk', >>> inlined from 'restore_user_regs.part.0' at arch/powerpc/kernel/signal_32.c:539:3: >>>>> include/linux/fortify-string.h:195:4: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] >>> 195 | __write_overflow_field(); >>> | ^~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> Cc: Michael Ellerman <mpe@ellerman.id.au> >>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >>> Cc: Paul Mackerras <paulus@samba.org> >>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu> >>> Cc: Sudeep Holla <sudeep.holla@arm.com> >>> Cc: linuxppc-dev@lists.ozlabs.org >>> Reported-by: kernel test robot <lkp@intel.com> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >>> arch/powerpc/include/asm/processor.h | 6 ++++-- >>> arch/powerpc/kernel/signal_32.c | 6 +++--- >>> 2 files changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h >>> index f348e564f7dd..05dc567cb9a8 100644 >>> --- a/arch/powerpc/include/asm/processor.h >>> +++ b/arch/powerpc/include/asm/processor.h >>> @@ -191,8 +191,10 @@ struct thread_struct { >>> int used_vsr; /* set if process has used VSX */ >>> #endif /* CONFIG_VSX */ >>> #ifdef CONFIG_SPE >>> - unsigned long evr[32]; /* upper 32-bits of SPE regs */ >>> - u64 acc; /* Accumulator */ >>> + struct_group(spe, >>> + unsigned long evr[32]; /* upper 32-bits of SPE regs */ >>> + u64 acc; /* Accumulator */ >>> + ); >>> unsigned long spefscr; /* SPE & eFP status */ >>> unsigned long spefscr_last; /* SPEFSCR value on last prctl >>> call or trap return */ >>> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c >>> index 0608581967f0..77b86caf5c51 100644 >>> --- a/arch/powerpc/kernel/signal_32.c >>> +++ b/arch/powerpc/kernel/signal_32.c >>> @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs, >>> regs_set_return_msr(regs, regs->msr & ~MSR_SPE); >>> if (msr & MSR_SPE) { >>> /* restore spe registers from the stack */ >>> - unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs, >>> - ELF_NEVRREG * sizeof(u32), failed); >>> + unsafe_copy_from_user(¤t->thread.spe, &sr->mc_vregs, >>> + sizeof(current->thread.spe), failed); >> >> This makes me nervous, because the ABI is that we copy ELF_NEVRREG * >> sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to >> be. >> >> ie. if we use sizeof an inadvertent change to the fields in >> thread_struct could change how many bytes we copy out to userspace, >> which would be an ABI break. >> >> And that's not that hard to do, because it's not at all obvious that the >> size and layout of fields in thread_struct affects the user ABI. >> >> At the same time we don't want to copy the right number of bytes but >> the wrong content, so from that point of view using sizeof is good :) >> >> The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that >> things match up, so maybe we should do that here too. >> >> ie. add: >> >> BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32)); > > You mean != I guess ? Gah. Yes I do :) cheers
On Thu, Aug 19, 2021 at 11:14:37AM -0700, Kees Cook wrote: > Which do you mean? When doing the conversions I tended to opt for > struct_group() since it provides more robust "intentionality". Strictly > speaking, the new memset helpers are doing field-spanning writes, but the > "clear to the end" pattern was so common it made sense to add the helpers, > as they're a bit less disruptive. It's totally up to you! :) Well, of the patches you cc'd to me only this one used the struct group.. Jason
On Tue, 17 Aug 2021, 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 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: Jiri Kosina <jikos@kernel.org> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Cc: linux-input@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Applied, thanks.
On Fri, Aug 20, 2021 at 03:01:43PM +0200, Jiri Kosina wrote: > On Tue, 17 Aug 2021, 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 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: Jiri Kosina <jikos@kernel.org> > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Cc: linux-input@vger.kernel.org > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Applied, thanks. I'm not sure if my other HTML email got through, but please don't apply these to separate trees -- struct_group() is introduced as part of this series.
On Fri, Aug 20, 2021 at 05:49:35PM +1000, Michael Ellerman wrote: > Kees Cook <keescook@chromium.org> writes: > > In preparation for FORTIFY_SOURCE performing compile-time and run-time > > field bounds checking for memset(), avoid intentionally writing across > > neighboring fields. > > > > Add a struct_group() for the spe registers so that memset() can correctly reason > > about the size: > > > > In function 'fortify_memset_chk', > > inlined from 'restore_user_regs.part.0' at arch/powerpc/kernel/signal_32.c:539:3: > >>> include/linux/fortify-string.h:195:4: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] > > 195 | __write_overflow_field(); > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > > Cc: Michael Ellerman <mpe@ellerman.id.au> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > Cc: Paul Mackerras <paulus@samba.org> > > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> > > Cc: Sudeep Holla <sudeep.holla@arm.com> > > Cc: linuxppc-dev@lists.ozlabs.org > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > arch/powerpc/include/asm/processor.h | 6 ++++-- > > arch/powerpc/kernel/signal_32.c | 6 +++--- > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h > > index f348e564f7dd..05dc567cb9a8 100644 > > --- a/arch/powerpc/include/asm/processor.h > > +++ b/arch/powerpc/include/asm/processor.h > > @@ -191,8 +191,10 @@ struct thread_struct { > > int used_vsr; /* set if process has used VSX */ > > #endif /* CONFIG_VSX */ > > #ifdef CONFIG_SPE > > - unsigned long evr[32]; /* upper 32-bits of SPE regs */ > > - u64 acc; /* Accumulator */ > > + struct_group(spe, > > + unsigned long evr[32]; /* upper 32-bits of SPE regs */ > > + u64 acc; /* Accumulator */ > > + ); > > unsigned long spefscr; /* SPE & eFP status */ > > unsigned long spefscr_last; /* SPEFSCR value on last prctl > > call or trap return */ > > diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c > > index 0608581967f0..77b86caf5c51 100644 > > --- a/arch/powerpc/kernel/signal_32.c > > +++ b/arch/powerpc/kernel/signal_32.c > > @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs, > > regs_set_return_msr(regs, regs->msr & ~MSR_SPE); > > if (msr & MSR_SPE) { > > /* restore spe registers from the stack */ > > - unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs, > > - ELF_NEVRREG * sizeof(u32), failed); > > + unsafe_copy_from_user(¤t->thread.spe, &sr->mc_vregs, > > + sizeof(current->thread.spe), failed); > > This makes me nervous, because the ABI is that we copy ELF_NEVRREG * > sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to > be. > > ie. if we use sizeof an inadvertent change to the fields in > thread_struct could change how many bytes we copy out to userspace, > which would be an ABI break. > > And that's not that hard to do, because it's not at all obvious that the > size and layout of fields in thread_struct affects the user ABI. > > At the same time we don't want to copy the right number of bytes but > the wrong content, so from that point of view using sizeof is good :) > > The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that > things match up, so maybe we should do that here too. > > ie. add: > > BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32)); > > Not sure if you are happy doing that as part of this patch. I can always > do it later if not. Sounds good to me; I did that in a few other cases in the series where the relationships between things seemed tenuous. :) I'll add this (as !=) in v3. Thanks!
On Fri, Aug 20, 2021 at 09:34:00AM -0300, Jason Gunthorpe wrote: > On Thu, Aug 19, 2021 at 11:14:37AM -0700, Kees Cook wrote: > > > Which do you mean? When doing the conversions I tended to opt for > > struct_group() since it provides more robust "intentionality". Strictly > > speaking, the new memset helpers are doing field-spanning writes, but the > > "clear to the end" pattern was so common it made sense to add the helpers, > > as they're a bit less disruptive. It's totally up to you! :) > > Well, of the patches you cc'd to me only this one used the struct > group.. Understood. I've adjusted this for v3. Thanks!
Kees Cook <keescook@chromium.org> writes: > On Fri, Aug 20, 2021 at 05:49:35PM +1000, Michael Ellerman wrote: >> Kees Cook <keescook@chromium.org> writes: >> > In preparation for FORTIFY_SOURCE performing compile-time and run-time >> > field bounds checking for memset(), avoid intentionally writing across >> > neighboring fields. >> > >> > Add a struct_group() for the spe registers so that memset() can correctly reason >> > about the size: >> > >> > In function 'fortify_memset_chk', >> > inlined from 'restore_user_regs.part.0' at arch/powerpc/kernel/signal_32.c:539:3: >> >>> include/linux/fortify-string.h:195:4: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning] >> > 195 | __write_overflow_field(); >> > | ^~~~~~~~~~~~~~~~~~~~~~~~ >> > >> > Cc: Michael Ellerman <mpe@ellerman.id.au> >> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> > Cc: Paul Mackerras <paulus@samba.org> >> > Cc: Christophe Leroy <christophe.leroy@csgroup.eu> >> > Cc: Sudeep Holla <sudeep.holla@arm.com> >> > Cc: linuxppc-dev@lists.ozlabs.org >> > Reported-by: kernel test robot <lkp@intel.com> >> > Signed-off-by: Kees Cook <keescook@chromium.org> >> > --- >> > arch/powerpc/include/asm/processor.h | 6 ++++-- >> > arch/powerpc/kernel/signal_32.c | 6 +++--- >> > 2 files changed, 7 insertions(+), 5 deletions(-) >> > >> > diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h >> > index f348e564f7dd..05dc567cb9a8 100644 >> > --- a/arch/powerpc/include/asm/processor.h >> > +++ b/arch/powerpc/include/asm/processor.h >> > @@ -191,8 +191,10 @@ struct thread_struct { >> > int used_vsr; /* set if process has used VSX */ >> > #endif /* CONFIG_VSX */ >> > #ifdef CONFIG_SPE >> > - unsigned long evr[32]; /* upper 32-bits of SPE regs */ >> > - u64 acc; /* Accumulator */ >> > + struct_group(spe, >> > + unsigned long evr[32]; /* upper 32-bits of SPE regs */ >> > + u64 acc; /* Accumulator */ >> > + ); >> > unsigned long spefscr; /* SPE & eFP status */ >> > unsigned long spefscr_last; /* SPEFSCR value on last prctl >> > call or trap return */ >> > diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c >> > index 0608581967f0..77b86caf5c51 100644 >> > --- a/arch/powerpc/kernel/signal_32.c >> > +++ b/arch/powerpc/kernel/signal_32.c >> > @@ -532,11 +532,11 @@ static long restore_user_regs(struct pt_regs *regs, >> > regs_set_return_msr(regs, regs->msr & ~MSR_SPE); >> > if (msr & MSR_SPE) { >> > /* restore spe registers from the stack */ >> > - unsafe_copy_from_user(current->thread.evr, &sr->mc_vregs, >> > - ELF_NEVRREG * sizeof(u32), failed); >> > + unsafe_copy_from_user(¤t->thread.spe, &sr->mc_vregs, >> > + sizeof(current->thread.spe), failed); >> >> This makes me nervous, because the ABI is that we copy ELF_NEVRREG * >> sizeof(u32) bytes, not whatever sizeof(current->thread.spe) happens to >> be. >> >> ie. if we use sizeof an inadvertent change to the fields in >> thread_struct could change how many bytes we copy out to userspace, >> which would be an ABI break. >> >> And that's not that hard to do, because it's not at all obvious that the >> size and layout of fields in thread_struct affects the user ABI. >> >> At the same time we don't want to copy the right number of bytes but >> the wrong content, so from that point of view using sizeof is good :) >> >> The way we handle it in ptrace is to have BUILD_BUG_ON()s to verify that >> things match up, so maybe we should do that here too. >> >> ie. add: >> >> BUILD_BUG_ON(sizeof(current->thread.spe) == ELF_NEVRREG * sizeof(u32)); >> >> Not sure if you are happy doing that as part of this patch. I can always >> do it later if not. > > Sounds good to me; I did that in a few other cases in the series where > the relationships between things seemed tenuous. :) I'll add this (as > !=) in v3. Thanks. cheers
Hi, This patch series (based on next-20210816) implements stricter (no struct member overflows) bounds checking for memcpy(), memmove(), and memset() under CONFIG_FORTIFY_SOURCE. To quote a later patch in the series: tl;dr: In order to eliminate a large class of common buffer overflow flaws that continue to persist in the kernel, have memcpy() (under CONFIG_FORTIFY_SOURCE) perform bounds checking of the destination struct member when they have a known size. This would have caught all of the memcpy()-related buffer write overflow flaws identified in at least the last three years. As this series introduces various helpers and performs several phases of treewide cleanups, I'm expecting to carry this series in my tree, so I'd love to get some Reviews and Acks. Given the size, I've mostly aimed this series at various mailing lists, otherwise the CC size got really big. :) I have separated a few things off of this submission, as things continue to grow: - run-time bounds checking (which now has a better solution for unknown bounds besides "oh well") - __alloc_size enablement (sent separately[1]) - -Warray-bounds enablement (several cleanups have been sent) The remaining portions (here) haven't seen as much change, so I think it's better to get these compile-time portions landed first -- they're big enough. My intention is to put this in -next soon via my "overflow" tree, assuming people are happy with it. :) Another random thing of note, I'm using the "Enhanced-by:" tag to mean "this was improved by...", inspired by the existing "Fixed-by:" tag (for "folded fixes"). "Fixed-by:" seemed inaccurate (it wasn't broken; it is just better now), and "Suggested-by:" seemed too nebulous (what was suggested?). Thanks! -Kees [1] https://lore.kernel.org/lkml/20210818050841.2226600-1-keescook@chromium.org/ v2: - fix CC_IS_CLANG typo (nathan) - bug.h hunk moved later (ndesaulnier) - various commit log typos (ndesaulnier) - omap3isp: fix read source (gustavo) - struct_group can be variadic (rasmus) - add struct_group_tagged (dan williams) - move raw __struct_group() to UAPI (gregkh, dan vetter) - ibmvscsi: add BUILD_BUG_ON() size checks - net: use memset_after() instead of struct_group() (jakob) - swap some memset_after() to memset_startat() for readability - add missing __NO_FORTIFY to arm boot stub (lkp) - powerpc signal32 struct_group() added (lkp) - powerpc sata_fsl struct_group() added - add fortify failure condition arguments to warning funcs to assist debugging - add ray_cs compile-time fix - add flexcan compile-time fix - add af_iucv compile-time fix - fix dropped strcpy() compile-time write overflow check - add missing strscpy() compile-time read overflow check - fix more boot stub build instances where fortification must be disabled - add reviewed-bys/acks v1: https://lore.kernel.org/lkml/20210727205855.411487-1-keescook@chromium.org/ Specifically, this series is logically split into several steps: Clean up remaining simple compile-time memcpy() warnings: ipw2x00: Avoid field-overflowing memcpy() net/mlx5e: Avoid field-overflowing memcpy() rpmsg: glink: Replace strncpy() with strscpy_pad() pcmcia: ray_cs: Split memcpy() to avoid bounds check warning Introduce struct_group() and apply it treewide to avoid compile-time memcpy() warnings: stddef: Introduce struct_group() helper macro cxl/core: Replace unions with struct_group() skbuff: Switch structure bounds to struct_group() bnxt_en: Use struct_group_attr() for memcpy() region mwl8k: Use struct_group() for memcpy() region libertas: Use struct_group() for memcpy() region libertas_tf: Use struct_group() for memcpy() region thermal: intel: int340x_thermal: Use struct_group() for memcpy() region iommu/amd: Use struct_group() for memcpy() region cxgb3: Use struct_group() for memcpy() region intersil: Use struct_group() for memcpy() region cxgb4: Use struct_group() for memcpy() region bnx2x: Use struct_group() for memcpy() region drm/amd/pm: Use struct_group() for memcpy() region staging: wlan-ng: Use struct_group() for memcpy() region drm/mga/mga_ioc32: Use struct_group() for memcpy() region net/mlx5e: Use struct_group() for memcpy() region HID: cp2112: Use struct_group() for memcpy() region media: omap3isp: Use struct_group() for memcpy() region sata_fsl: Use struct_group() for memcpy() region Prepare fortify for additional hardening: compiler_types.h: Remove __compiletime_object_size() lib/string: Move helper functions out of string.c fortify: Move remaining fortify helpers into fortify-string.h fortify: Explicitly disable Clang support fortify: Fix dropped strcpy() compile-time write overflow check fortify: Prepare to improve strnlen() and strlen() warnings fortify: Allow strlen() and strnlen() to pass compile-time known lengths Add compile-time and run-time tests: fortify: Add compile-time FORTIFY_SOURCE tests lib: Introduce CONFIG_TEST_MEMCPY Enable new compile-time memcpy() and memmove() bounds checking: fortify: Detect struct member overflows in memcpy() at compile-time fortify: Detect struct member overflows in memmove() at compile-time Clean up remaining simple compile-time memset() warnings: scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp Introduce memset_after() and memset_startat() helpers and apply them (and struct_group()): string.h: Introduce memset_after() for wiping trailing members/padding xfrm: Use memset_after() to clear padding ipv6: Use memset_after() to zero rt6_info netfilter: conntrack: Use memset_startat() to zero struct nf_conn net: 802: Use memset_startat() to clear struct fields net: dccp: Use memset_startat() for TP zeroing net: qede: Use memset_startat() for counters mac80211: Use memset_after() to clear tx status ath11k: Use memset_startat() for clearing queue descriptors iw_cxgb4: Use memset_startat() for cpl_t5_pass_accept_rpl intel_th: msu: Use memset_startat() for clearing hw header IB/mthca: Use memset_startat() for clearing mpt_entry btrfs: Use memset_startat() to clear end of struct tracing: Use memset_startat() to zero struct trace_iterator drbd: Use struct_group() to zero algs cm4000_cs: Use struct_group() to zero struct cm4000_dev region KVM: x86: Use struct_group() to zero decode cache dm integrity: Use struct_group() to zero struct journal_sector HID: roccat: Use struct_group() to zero kone_mouse_event RDMA/mlx5: Use struct_group() to zero struct mlx5_ib_mr powerpc/signal32: Use struct_group() to zero spe regs ethtool: stats: Use struct_group() to clear all stats at once can: flexcan: Use struct_group() to zero struct flexcan_regs regions net/af_iucv: Use struct_group() to zero struct iucv_sock region powerpc: Split memset() to avoid multi-field overflow Enable new compile-time memset() bounds checking: fortify: Detect struct member overflows in memset() at compile-time Enable Clang support: fortify: Work around Clang inlining bugs arch/arm/boot/compressed/string.c | 1 + arch/powerpc/include/asm/processor.h | 6 +- arch/powerpc/kernel/signal_32.c | 6 +- arch/s390/lib/string.c | 3 + arch/x86/boot/compressed/misc.c | 3 +- arch/x86/boot/compressed/misc.h | 2 + arch/x86/boot/compressed/pgtable_64.c | 2 + arch/x86/kvm/emulate.c | 3 +- arch/x86/kvm/kvm_emulate.h | 19 +- arch/x86/lib/memcpy_32.c | 1 + arch/x86/lib/string_32.c | 1 + drivers/ata/sata_fsl.c | 10 +- drivers/block/drbd/drbd_main.c | 3 +- drivers/block/drbd/drbd_protocol.h | 6 +- drivers/block/drbd/drbd_receiver.c | 3 +- drivers/char/pcmcia/cm4000_cs.c | 9 +- drivers/cxl/cxl.h | 61 +--- drivers/gpu/drm/amd/include/atomfirmware.h | 9 +- .../drm/amd/pm/inc/smu11_driver_if_arcturus.h | 3 +- .../drm/amd/pm/inc/smu11_driver_if_navi10.h | 3 +- .../amd/pm/inc/smu13_driver_if_aldebaran.h | 3 +- .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 6 +- .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 12 +- .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 6 +- drivers/gpu/drm/mga/mga_ioc32.c | 27 +- drivers/hid/hid-cp2112.c | 14 +- drivers/hid/hid-roccat-kone.c | 2 +- drivers/hid/hid-roccat-kone.h | 12 +- drivers/hwtracing/intel_th/msu.c | 4 +- drivers/infiniband/hw/cxgb4/cm.c | 5 +- drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +- drivers/infiniband/hw/mthca/mthca_mr.c | 3 +- drivers/iommu/amd/init.c | 9 +- drivers/macintosh/smu.c | 3 +- drivers/md/dm-integrity.c | 9 +- drivers/media/platform/omap3isp/ispstat.c | 5 +- drivers/net/can/flexcan.c | 68 ++-- .../net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 7 +- .../net/ethernet/broadcom/bnx2x/bnx2x_stats.h | 14 +- drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 4 +- drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.h | 14 +- drivers/net/ethernet/chelsio/cxgb3/sge.c | 9 +- drivers/net/ethernet/chelsio/cxgb4/sge.c | 8 +- drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 2 +- drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 10 +- drivers/net/ethernet/chelsio/cxgb4vf/sge.c | 7 +- drivers/net/ethernet/mellanox/mlx5/core/en.h | 4 +- .../net/ethernet/mellanox/mlx5/core/en/xdp.c | 4 +- .../net/ethernet/mellanox/mlx5/core/en_tx.c | 2 +- drivers/net/ethernet/qlogic/qede/qede_main.c | 2 +- drivers/net/wireguard/queueing.h | 4 +- drivers/net/wireless/ath/ath11k/hal_rx.c | 13 +- drivers/net/wireless/ath/carl9170/tx.c | 11 +- .../net/wireless/intel/ipw2x00/libipw_rx.c | 56 +--- .../net/wireless/intersil/hostap/hostap_hw.c | 5 +- .../wireless/intersil/hostap/hostap_wlan.h | 14 +- drivers/net/wireless/intersil/p54/txrx.c | 6 +- drivers/net/wireless/marvell/libertas/host.h | 10 +- drivers/net/wireless/marvell/libertas/tx.c | 5 +- .../marvell/libertas_tf/libertas_tf.h | 10 +- .../net/wireless/marvell/libertas_tf/main.c | 3 +- drivers/net/wireless/marvell/mwl8k.c | 10 +- drivers/net/wireless/ray_cs.c | 4 +- drivers/rpmsg/qcom_glink_native.c | 2 +- drivers/scsi/ibmvscsi/ibmvscsi.c | 3 +- drivers/staging/wlan-ng/hfa384x.h | 16 +- drivers/staging/wlan-ng/hfa384x_usb.c | 4 +- .../intel/int340x_thermal/acpi_thermal_rel.c | 5 +- .../intel/int340x_thermal/acpi_thermal_rel.h | 48 +-- fs/btrfs/root-tree.c | 6 +- include/linux/compiler-gcc.h | 2 - include/linux/compiler_types.h | 4 - include/linux/fortify-string.h | 308 +++++++++++++----- include/linux/if_vlan.h | 6 +- include/linux/skbuff.h | 9 +- include/linux/stddef.h | 47 +++ include/linux/string.h | 43 ++- include/linux/thread_info.h | 2 +- include/net/iucv/af_iucv.h | 10 +- include/net/mac80211.h | 7 +- include/uapi/drm/mga_drm.h | 22 +- include/uapi/linux/omap3isp.h | 21 +- include/uapi/linux/stddef.h | 21 ++ kernel/trace/trace.c | 4 +- lib/.gitignore | 2 + lib/Kconfig.debug | 7 + lib/Makefile | 35 ++ lib/string.c | 210 +----------- lib/string_helpers.c | 201 ++++++++++++ lib/test_fortify/read_overflow-memchr.c | 5 + lib/test_fortify/read_overflow-memchr_inv.c | 5 + lib/test_fortify/read_overflow-memcmp.c | 5 + lib/test_fortify/read_overflow-memscan.c | 5 + lib/test_fortify/read_overflow2-memcmp.c | 5 + lib/test_fortify/read_overflow2-memcpy.c | 5 + lib/test_fortify/read_overflow2-memmove.c | 5 + .../read_overflow2_field-memcpy.c | 5 + .../read_overflow2_field-memmove.c | 5 + lib/test_fortify/test_fortify.h | 35 ++ lib/test_fortify/write_overflow-memcpy.c | 5 + lib/test_fortify/write_overflow-memmove.c | 5 + lib/test_fortify/write_overflow-memset.c | 5 + lib/test_fortify/write_overflow-strcpy-lit.c | 5 + lib/test_fortify/write_overflow-strcpy.c | 5 + lib/test_fortify/write_overflow-strlcpy-src.c | 5 + lib/test_fortify/write_overflow-strlcpy.c | 5 + lib/test_fortify/write_overflow-strncpy-src.c | 5 + lib/test_fortify/write_overflow-strncpy.c | 5 + lib/test_fortify/write_overflow-strscpy.c | 5 + .../write_overflow_field-memcpy.c | 5 + .../write_overflow_field-memmove.c | 5 + .../write_overflow_field-memset.c | 5 + lib/test_memcpy.c | 288 ++++++++++++++++ net/802/hippi.c | 2 +- net/core/skbuff.c | 14 +- net/dccp/trace.h | 4 +- net/ethtool/stats.c | 15 +- net/ipv6/route.c | 4 +- net/iucv/af_iucv.c | 2 +- net/netfilter/nf_conntrack_core.c | 4 +- net/xfrm/xfrm_policy.c | 4 +- net/xfrm/xfrm_user.c | 2 +- scripts/test_fortify.sh | 59 ++++ security/Kconfig | 3 + 124 files changed, 1489 insertions(+), 686 deletions(-) create mode 100644 lib/test_fortify/read_overflow-memchr.c create mode 100644 lib/test_fortify/read_overflow-memchr_inv.c create mode 100644 lib/test_fortify/read_overflow-memcmp.c create mode 100644 lib/test_fortify/read_overflow-memscan.c create mode 100644 lib/test_fortify/read_overflow2-memcmp.c create mode 100644 lib/test_fortify/read_overflow2-memcpy.c create mode 100644 lib/test_fortify/read_overflow2-memmove.c create mode 100644 lib/test_fortify/read_overflow2_field-memcpy.c create mode 100644 lib/test_fortify/read_overflow2_field-memmove.c create mode 100644 lib/test_fortify/test_fortify.h create mode 100644 lib/test_fortify/write_overflow-memcpy.c create mode 100644 lib/test_fortify/write_overflow-memmove.c create mode 100644 lib/test_fortify/write_overflow-memset.c create mode 100644 lib/test_fortify/write_overflow-strcpy-lit.c create mode 100644 lib/test_fortify/write_overflow-strcpy.c create mode 100644 lib/test_fortify/write_overflow-strlcpy-src.c create mode 100644 lib/test_fortify/write_overflow-strlcpy.c create mode 100644 lib/test_fortify/write_overflow-strncpy-src.c create mode 100644 lib/test_fortify/write_overflow-strncpy.c create mode 100644 lib/test_fortify/write_overflow-strscpy.c create mode 100644 lib/test_fortify/write_overflow_field-memcpy.c create mode 100644 lib/test_fortify/write_overflow_field-memmove.c create mode 100644 lib/test_fortify/write_overflow_field-memset.c create mode 100644 lib/test_memcpy.c create mode 100644 scripts/test_fortify.sh