Re: __builtin_dynamic_object_size

Re: __builtin_dynamic_object_size

[Richard Smith]

[Please CC me; I'm not subscribed]

On Thu, 24 Jan 2019 11:59 at Richard Biener wrote:
> On Wed, Jan 23, 2019 at 12:33 PM Jakub Jelinek <jakub@redhat.com> wrote:
> > On Wed, Jan 23, 2019 at 10:40:43AM +0000, Jonathan Wakely wrote:
> > > There's a patch to add __builtin_dynamic_object_size to clang:
> > > https://reviews.llvm.org/D56760
> > >
> > > It was suggested that this could be done via a new flag bit for
> > > __builtin_object_size, but only if GCC would support that too
> > > (otherwise it would be done as a separate builtin).
> > >
> > > Is there any interest in adding that as an option to __builtin_object_size?
> > >
> > > I know Jakub is concerned about arbitrarily complex expressions, when
> > > __builtin_object_size is supposed to always be efficient and always
> > > evaluate at compile time (which would imply the dynamic behaviour
> > > should be a separate builtin, if it exists at all).
> >
> > The current modes (0-3) certainly must not be changed and must return a
> > constant, that is what huge amounts of code in the wild relies on.
>
> I wouldn't overload _bos but only use a new builtin.

Clang provides another useful attribute in this space:
__attribute__((pass_object_size(N))), when applied to a pointer
parameter to a function, causes the compiler to evaluate
__builtin_object_size in the caller, and pass the result to the callee
(see https://clang.llvm.org/docs/AttributeReference.html#pass-object-size).
This allows many of the _FORTIFY_SOURCE checks to be implemented
without forcibly inlining. One reason we'd like to see this added as a
flag bit rather than as a separate builtin is that it would then also
naturally extend to the pass_object_size attribute. But we certainly
don't want to conflict with any potential future GCC extension.

If the GCC devs aren't interested in adding similar functionality,
could we reserve a bit for this behavior? Eg, if GCC would promise to
never use the values 4-7 for any other purpose, I expect that'd work
for us, but I'd certainly understand if you don't want to guarantee
that.

> > The reason to choose constants only was the desire to make _FORTIFY_SOURCE
> > cheap at runtime.  For the dynamically computed expressions, the question
> > is how far it should go, how complex expressions it wants to build and how
> > much code and runtime can be spent on computing that.
> >
> > The rationale for __builtin_dynamic_object_size lists only very simple
> > cases, where the builtin is just called on result of malloc, so that is
> > indeed easy, the argument is already evaluated before the malloc call, so
> > you can just save it in a temporary and use later.  Slightly more complex
> > is calloc, where you need to multiply two numbers (do you handle overflow
> > somehow, or not?).  But in real world, it can be arbitrarily more complex,
> > there can be pointer arithmetics with constant or variable offsets,
> > there can be conditional adjustments of pointers or PHIs with multiple
> > "dynamic" expressions for the sizes (shall the dynamic expression evaluate
> > as max over expressions for different phi arguments (that is essentially
> > what is done for the constant __builtin_object_size, but for dynamic
> > expressions max needs to be computed at runtime, or shall it reconstruct
> > the conditional or remember it and compute whatever ? val1 : val2),
> > loops which adjust pointers, etc. and all that can be done many times in
> > between where the objects are allocated and where the builtin is used.
>
> Which means I'd like to see a thorough specification of the builtin.

How far you go is a quality of implementation issue, just like with
all the existing __builtin_object_size modes -- you're always
permitted to return a conservatively-correct answer, for any mode.
(Ignoring the new flag bit would be a correct implementation, for
example.) But the intent is for this to be used in cases where users
want the security benefits more than they want a modicum of
performance.

> If it is allowed to return "failure" in any event then of what use is
> the builtin in practice?

The same question can be asked of the existing builtin, which has the
same property that it is allowed to return "failure" in any event.
That's already useful in practice, and it's hopefully also apparent
that catching just the easy non-constant cases (eg, a parameter passed
to malloc also gets used as the object size of the returned pointer)
would also be useful in practice.

Re: __builtin_dynamic_object_size

[Richard Biener]

On Wed, Jan 23, 2019 at 12:33 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Wed, Jan 23, 2019 at 10:40:43AM +0000, Jonathan Wakely wrote:
> > There's a patch to add __builtin_dynamic_object_size to clang:
> > https://reviews.llvm.org/D56760
> >
> > It was suggested that this could be done via a new flag bit for
> > __builtin_object_size, but only if GCC would support that too
> > (otherwise it would be done as a separate builtin).
> >
> > Is there any interest in adding that as an option to __builtin_object_size?
> >
> > I know Jakub is concerned about arbitrarily complex expressions, when
> > __builtin_object_size is supposed to always be efficient and always
> > evaluate at compile time (which would imply the dynamic behaviour
> > should be a separate builtin, if it exists at all).
>
> The current modes (0-3) certainly must not be changed and must return a
> constant, that is what huge amounts of code in the wild relies on.

I wouldn't overload _bos but only use a new builtin.

> The reason to choose constants only was the desire to make _FORTIFY_SOURCE
> cheap at runtime.  For the dynamically computed expressions, the question
> is how far it should go, how complex expressions it wants to build and how
> much code and runtime can be spent on computing that.
>
> The rationale for __builtin_dynamic_object_size lists only very simple
> cases, where the builtin is just called on result of malloc, so that is
> indeed easy, the argument is already evaluated before the malloc call, so
> you can just save it in a temporary and use later.  Slightly more complex
> is calloc, where you need to multiply two numbers (do you handle overflow
> somehow, or not?).  But in real world, it can be arbitrarily more complex,
> there can be pointer arithmetics with constant or variable offsets,
> there can be conditional adjustments of pointers or PHIs with multiple
> "dynamic" expressions for the sizes (shall the dynamic expression evaluate
> as max over expressions for different phi arguments (that is essentially
> what is done for the constant __builtin_object_size, but for dynamic
> expressions max needs to be computed at runtime, or shall it reconstruct
> the conditional or remember it and compute whatever ? val1 : val2),
> loops which adjust pointers, etc. and all that can be done many times in
> between where the objects are allocated and where the builtin is used.

Which means I'd like to see a thorough specification of the builtin.
If it is allowed to return "failure" in any event then of what use is
the builtin in practice?

Richard.

>         Jakub

Re: __builtin_dynamic_object_size

[Martin Sebor]

On 1/23/19 3:40 AM, Jonathan Wakely wrote:
> There's a patch to add __builtin_dynamic_object_size to clang:
> https://reviews.llvm.org/D56760
> 
> It was suggested that this could be done via a new flag bit for
> __builtin_object_size, but only if GCC would support that too
> (otherwise it would be done as a separate builtin).
> 
> Is there any interest in adding that as an option to __builtin_object_size?
> 
> I know Jakub is concerned about arbitrarily complex expressions, when
> __builtin_object_size is supposed to always be efficient and always
> evaluate at compile time (which would imply the dynamic behaviour
> should be a separate builtin, if it exists at all).

I am very interested in doing something like that and handling at
least the simple cases (with minimum runtime overhead).  I haven't
thought about it hard enough to have a clear idea whether it needs
a new built-in or whether the current one can be extended to handle
non-constant cases as well (perhaps by adding a new bit) but I would
certainly want the existing libc infrastructure to make use of
the non-constant sizes without having to change.  The overhead of
handling the more complex cases that Jakub is concerned about could
be controlled by some customizable parameter so I don't think that
should stand in the way.

Besides handling non-constant object sizes I would also like GCC to
get better about detecting and preventing subobject overflow (such
as in strcpy (s.m, "foobar") where GCC transforms the strcpy call
to memcpy which is allowed to overwrite whatever follows s.m).

Martin

Re: __builtin_dynamic_object_size

[Jakub Jelinek]

On Wed, Jan 23, 2019 at 10:40:43AM +0000, Jonathan Wakely wrote:
> There's a patch to add __builtin_dynamic_object_size to clang:
> https://reviews.llvm.org/D56760
> 
> It was suggested that this could be done via a new flag bit for
> __builtin_object_size, but only if GCC would support that too
> (otherwise it would be done as a separate builtin).
> 
> Is there any interest in adding that as an option to __builtin_object_size?
> 
> I know Jakub is concerned about arbitrarily complex expressions, when
> __builtin_object_size is supposed to always be efficient and always
> evaluate at compile time (which would imply the dynamic behaviour
> should be a separate builtin, if it exists at all).

The current modes (0-3) certainly must not be changed and must return a
constant, that is what huge amounts of code in the wild relies on.

The reason to choose constants only was the desire to make _FORTIFY_SOURCE
cheap at runtime.  For the dynamically computed expressions, the question
is how far it should go, how complex expressions it wants to build and how
much code and runtime can be spent on computing that.

The rationale for __builtin_dynamic_object_size lists only very simple
cases, where the builtin is just called on result of malloc, so that is
indeed easy, the argument is already evaluated before the malloc call, so
you can just save it in a temporary and use later.  Slightly more complex
is calloc, where you need to multiply two numbers (do you handle overflow
somehow, or not?).  But in real world, it can be arbitrarily more complex,
there can be pointer arithmetics with constant or variable offsets,
there can be conditional adjustments of pointers or PHIs with multiple
"dynamic" expressions for the sizes (shall the dynamic expression evaluate
as max over expressions for different phi arguments (that is essentially
what is done for the constant __builtin_object_size, but for dynamic
expressions max needs to be computed at runtime, or shall it reconstruct
the conditional or remember it and compute whatever ? val1 : val2),
loops which adjust pointers, etc. and all that can be done many times in
between where the objects are allocated and where the builtin is used.

	Jakub

__builtin_dynamic_object_size

[Jonathan Wakely]

There's a patch to add __builtin_dynamic_object_size to clang:
https://reviews.llvm.org/D56760

It was suggested that this could be done via a new flag bit for
__builtin_object_size, but only if GCC would support that too
(otherwise it would be done as a separate builtin).

Is there any interest in adding that as an option to __builtin_object_size?

I know Jakub is concerned about arbitrarily complex expressions, when
__builtin_object_size is supposed to always be efficient and always
evaluate at compile time (which would imply the dynamic behaviour
should be a separate builtin, if it exists at all).