Messages in this thread | | | Date | Mon, 30 Aug 2021 13:12:28 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds |
| |
On Mon, Aug 30, 2021 at 11:44:54AM -0700, Nathan Chancellor wrote: > On Fri, Aug 27, 2021 at 09:30:10AM -0700, Kees Cook wrote: > > v3: > > - fix typo in treewide conversion (u8 should have been __u8) > > - improve changelog for DECLARE_FLEX_ARRAY patch > > - add acks/reviews > > v2: https://lore.kernel.org/lkml/20210826050458.1540622-1-keescook@chromium.org/ > > v1: https://lore.kernel.org/lkml/20210818081118.1667663-1-keescook@chromium.org/ > > > > Hi, > > > > In support of the improved buffer overflow detection for memcpy(), > > this enables -Warray-bounds and -Wzero-length-bounds globally. Mostly > > it involves some struct member tricks with the new DECLARE_FLEX_ARRAY() > > macro. Everything else is just replacing stacked 0-element arrays > > with actual unions in two related treewide patches. There is one set of > > special cases that were fixed separately[1] and are needed as well. > > > > I'm expecting to carry this series with the memcpy() series in my > > "overflow" tree. Reviews appreciated! :) > > Hi Kees, > > I ran this series through my local build tests and uncovered two > warnings in the same file that appear to be unhandled as of > next-20210830. This is from ARCH=powerpc pseries_defconfig with > clang-14, I did not try earlier versions of clang.
Thanks for double-checking!
> > arch/powerpc/kernel/signal_32.c:780:2: error: array index 3 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds] > unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > [...] > arch/powerpc/kernel/signal_32.c:1044:3: error: array index 2 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds] > unsafe_put_sigset_t(&old_ctx->uc_sigmask, ¤t->blocked, failed); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This smells like some kind of casting issue. uc_sigmask has only a single unsigned long element but unsafe_put_compat_sigset() seems to be doing stuff with [3], etc. Is it expecting u8? I will keep looking...
-- Kees Cook
| |