All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Siddhesh Poyarekar <siddhesh@gotplt.org>
Cc: linux-hardening@vger.kernel.org,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Arnd Bergmann <arnd@arndb.de>, Juergen Gross <jgross@suse.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Tom Rix <trix@redhat.com>, Miguel Ojeda <ojeda@kernel.org>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH 2/4] fortify: Explicitly check bounds are compile-time constants
Date: Wed, 21 Sep 2022 20:46:47 -0700	[thread overview]
Message-ID: <202209212034.16D9025882@keescook> (raw)
In-Reply-To: <b8f5f716-3ee6-87fd-d0e2-b1c35c98e0b0@gotplt.org>

On Wed, Sep 21, 2022 at 07:48:44AM -0400, Siddhesh Poyarekar wrote:
> On 2022-09-20 15:22, Kees Cook wrote:
> > In preparation for replacing __builtin_object_size() with
> > __builtin_dynamic_object_size(), all the compile-time size checks need
> > to check that the bounds variables are, in fact, known at compile-time.
> > Enforce what was guaranteed with __bos(). In other words, since all uses
> > of __bos() were constant expressions, it was not required to test for
> > this. When these change to __bdos(), they _may_ be constant expressions,
> > and the checks are only valid when the prior condition holds. This
> > results in no binary differences.
> > 
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >   include/linux/fortify-string.h | 50 +++++++++++++++++++++-------------
> >   1 file changed, 31 insertions(+), 19 deletions(-)
> > 
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index ff879efe94ed..71c0a432c638 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -80,6 +80,12 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
> >   #define POS	__pass_object_size(1)
> >   #define POS0	__pass_object_size(0)
> > +#define __compiletime_lessthan(bounds, length)	(	\
> > +	__builtin_constant_p(length) &&			\
> > +	__builtin_constant_p(bounds) &&			\
> > +	bounds < length					\
> > +)
> 
> So with the gcc ranger, the compiler has lately been quite successful at
> computing a constant `bounds < length` even though bounds and length are not
> constant.  So perhaps this:
> 
> #define __compiletime_lessthan (bounds, length) (	\
> 	__builtin_constant (bounds < length) &&		\
> 	bounds < length					\
> )
> 
> might succeed in a few more cases.

Oh, interesting! That's very cool -- I never considered tossing a full
expression into __bcp. Yeah, that seems to work just fine:
https://godbolt.org/z/xrchErEx1

-- 
Kees Cook

  reply	other threads:[~2022-09-22  3:46 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-20 19:21 [PATCH 0/4] fortify: Use __builtin_dynamic_object_size() when available Kees Cook
2022-09-20 19:21 ` [PATCH 1/4] x86/entry: Work around Clang __bdos() bug Kees Cook
2022-09-21  0:07   ` Boris Ostrovsky
2022-09-20 19:22 ` [PATCH 2/4] fortify: Explicitly check bounds are compile-time constants Kees Cook
2022-09-21 11:48   ` Siddhesh Poyarekar
2022-09-22  3:46     ` Kees Cook [this message]
2022-09-20 19:22 ` [PATCH 3/4] fortify: Convert to struct vs member helpers Kees Cook
2022-09-20 19:22 ` [PATCH 4/4] fortify: Use __builtin_dynamic_object_size() when available Kees Cook
2022-09-21 11:24   ` Miguel Ojeda
2022-09-21 11:43   ` Siddhesh Poyarekar
2022-09-22  3:33     ` Kees Cook
2022-09-22 14:45       ` Siddhesh Poyarekar
2022-11-22 10:20   ` Siddhesh Poyarekar
2022-11-23  5:15     ` Kees Cook
2022-11-23 15:29       ` Siddhesh Poyarekar
2023-01-13 15:59   ` linux-next - bxnt buffer overflow in strnlen Niklas Cassel
2023-01-13 16:08     ` linux-next - bnxt " Niklas Cassel
2023-01-13 22:44       ` Kees Cook
2023-01-16 10:56         ` Niklas Cassel
2022-09-22 20:26 ` [PATCH 0/4] fortify: Use __builtin_dynamic_object_size() when available Siddhesh Poyarekar
2022-09-23  0:20   ` Kees Cook
2022-09-23  0:55     ` Siddhesh Poyarekar

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=202209212034.16D9025882@keescook \
    --to=keescook@chromium.org \
    --cc=arnd@arndb.de \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jgross@suse.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=siddhesh@gotplt.org \
    --cc=trix@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.