All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	llvm@lists.linux.dev, linux-hardening@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Miguel Ojeda <ojeda@kernel.org>, Marco Elver <elver@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/5] overflow: Expand check_add_overflow() for pointer addition
Date: Fri, 2 Feb 2024 01:04:49 -0800	[thread overview]
Message-ID: <202402020102.FDD94EBE2@keescook> (raw)
In-Reply-To: <ac88504b-1edc-46c5-ae61-7a634b156275@intel.com>

On Thu, Feb 01, 2024 at 10:19:15AM +0100, Przemek Kitszel wrote:
> On 1/30/24 23:06, Kees Cook wrote:
> > The check_add_overflow() helper is mostly a wrapper around
> > __builtin_add_overflow(), but GCC and Clang refuse to operate on pointer
> > arguments that would normally be allowed if the addition were open-coded.
> > 
> > For example, we have many places where pointer overflow is tested:
> > 
> > 	struct foo *ptr;
> > 	...
> > 	/* Check for overflow */
> > 	if (ptr + count < ptr) ...
> > 
> > And in order to avoid running into the overflow sanitizers in the
> > future, we need to rewrite these "intended" overflow checks:
> > 
> > 	if (check_add_overflow(ptr, count, &result)) ...
> > 
> > Frustratingly the argument type validation for __builtin_add_overflow()
> > is done before evaluating __builtin_choose_expr(), so for arguments to
> > be valid simultaneously for sizeof(*p) (when p may not be a pointer),
> > and __builtin_add_overflow(a, ...) (when a may be a pointer), we must
> > introduce wrappers that always produce a specific type (but they are
> > only used in the places where the bogus arguments will be ignored).
> > 
> > To test whether a variable is a pointer or not, introduce the __is_ptr()
> > helper, which uses __builtin_classify_type() to find arrays and pointers
> > (via the new __is_ptr_or_array() helper), and then decays arrays into
> > pointers (via the new __decay() helper), to distinguish pointers from
> > arrays.
> 
> This is (not just commit msg but together with impl), at first glance, too
> complicated for regular developers to grasp (that is perhaps fine),
> but could we make it simpler by, say _Generic() or other trick?

I haven't been able to find a way to do this, unfortunately. :( I would
*love* to find something simpler, but it eludes me.

-- 
Kees Cook

  reply	other threads:[~2024-02-02  9:04 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-30 22:06 [PATCH v2 0/5] overflow: Introduce wrapping helpers Kees Cook
2024-01-30 22:06 ` [PATCH v2 1/5] overflow: Adjust check_*_overflow() kern-doc to reflect results Kees Cook
2024-01-30 22:06 ` [PATCH v2 2/5] overflow: Expand check_add_overflow() for pointer addition Kees Cook
2024-01-31  8:35   ` Rasmus Villemoes
2024-02-02  9:26     ` Kees Cook
2024-02-01  9:19   ` Przemek Kitszel
2024-02-02  9:04     ` Kees Cook [this message]
2024-01-30 22:06 ` [PATCH v2 3/5] overflow: Introduce add_would_overflow() Kees Cook
2024-01-30 22:06 ` [PATCH v2 4/5] overflow: Introduce add_wrap(), sub_wrap(), and mul_wrap() Kees Cook
2024-01-30 22:06 ` [PATCH v2 5/5] overflow: Introduce inc_wrap() and dec_wrap() 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=202402020102.FDD94EBE2@keescook \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=elver@google.com \
    --cc=gustavoars@kernel.org \
    --cc=justinstitt@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=rasmus.villemoes@prevas.dk \
    /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.