Re: [PATCH for-next 19/25] fortify: Allow strlen() and strnlen() to pass compile-time known lengths

From: Nick Desaulniers <ndesaulniers@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kernel@vger.kernel.org,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Daniel Micay <danielmicay@gmail.com>,
	Francis Laniel <laniel_francis@privacyrequired.com>,
	Bart Van Assche <bvanassche@acm.org>,
	David Gow <davidgow@google.com>,
	linux-mm@kvack.org, clang-built-linux@googlegroups.com,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH for-next 19/25] fortify: Allow strlen() and strnlen() to pass compile-time known lengths
Date: Thu, 26 Aug 2021 11:02:39 -0700	[thread overview]
Message-ID: <CAKwvOdndEYeQ1MFPPEvn=UWv00o6of1OT3UQ-woqyO-viDkqVw@mail.gmail.com> (raw)
In-Reply-To: <202108251950.61F7A4CD@keescook>

On Wed, Aug 25, 2021 at 7:56 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Aug 25, 2021 at 03:05:56PM -0700, Nick Desaulniers wrote:
> > On Sun, Aug 22, 2021 at 12:57 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > > index a3cb1d9aacce..e232a63fd826 100644
> > > --- a/include/linux/fortify-string.h
> > > +++ b/include/linux/fortify-string.h
> > > @@ -10,6 +10,18 @@ void __read_overflow(void) __compiletime_error("detected read beyond size of obj
> > >  void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
> > >  void __write_overflow(void) __compiletime_error("detected write beyond size of object (1st parameter)");
> > >
> > > +#define __compiletime_strlen(p)        ({              \
> > > +       size_t ret = (size_t)-1;                        \
> > > +       size_t p_size = __builtin_object_size(p, 1);    \
> > > +       if (p_size != (size_t)-1) {                     \
> > > +               size_t p_len = p_size - 1;              \
> > > +               if (__builtin_constant_p(p[p_len]) &&   \
> > > +                   p[p_len] == '\0')                   \
> > > +                       ret = __builtin_strlen(p);      \
> > > +       }                                               \
> > > +       ret;                                            \
> > > +})
> >
> > Can this be a `static inline` function that accepts a `const char *`
> > and returns a `size_t` rather than a statement expression?
>
> No because both __builtin_object_size() and __builtin_strlen() may not
> work. See:
> https://lore.kernel.org/lkml/20210818060533.3569517-64-keescook@chromium.org/

Ah right, then consider adding a comment to encourage others not to
rewrite it as such.

>
> Regardless, it will always collapse to a const value of either -1 or
> the length of the string.

-- 
Thanks,
~Nick Desaulniers