On Thu, May 3, 2018 at 4:00 PM, Rasmus Villemoes
[off-list ref] wrote:

On 2018-05-01 19:00, Kees Cook wrote:

quoted

On Mon, Apr 30, 2018 at 2:29 PM, Rasmus Villemoes
[off-list ref] wrote:

quoted

gcc 5.1+ (I think) have the __builtin_OP_overflow checks that should
generate reasonable code. Too bad there's no completely generic
check_all_ops_in_this_expression(a+b*c+d/e, or_jump_here). Though it's
hard to define what they should be checked against - probably would
require all subexpressions (including the variables themselves) to have
the same type.

plug: https://lkml.org/lkml/2015/7/19/358

That's a very nice series. Why did it never get taken?

Well, nobody seemed particularly interested, and then
https://lkml.org/lkml/2015/10/28/215 happened... but he did later seem
to admit that it could be useful for the multiplication checking, and
that "the gcc interface for multiplication overflow is fine".

Oh, excellent. Thank you for that pointer! That conversation covered a
lot of ground. I need to think a little more about how to apply the
thoughts there with the kmalloc() needs and the GPU driver needs...

I still think even for unsigned types overflow checking can be subtle. E.g.

u32 somevar;

if (somevar + sizeof(foo) < somevar)
  return -EOVERFLOW;
somevar += sizeof(this);

is broken, because the LHS is promoted to unsigned long/size_t, then so
is the RHS for the comparison, and the comparison is thus always false
(on 64bit). It gets worse if the two types are more "opaque", and in any
case it's not always easy to verify at a glance that the types are the
same, or at least that the expression of the widest type is on the RHS.

That's an excellent example, yes. (And likely worth including in the
commit log somewhere.)

quoted

It seems to do the right things quite correctly.

Yes, I wouldn't suggest it without the test module verifying corner
cases, and checking it has the same semantics whether used with old or
new gcc.

Would you shepherd it through if I updated the patches and resent?

Yes, though we may need reworking if we actually want to do the
try/catch style (since that was talked about with GPU stuff too...)

Either way, yes, a refresh would be lovely! :)

-Kees

-- 
Kees Cook
Pixel Security