Date: Fri, 4 May 2018 01:00:43 +0200 From: Rasmus Villemoes <linux@...musvillemoes.dk> To: Kees Cook <keescook@...omium.org>, Daniel Vetter <daniel.vetter@...el.com> Cc: Matthew Wilcox <willy@...radead.org>, Julia Lawall <julia.lawall@...6.fr>, Andrew Morton <akpm@...ux-foundation.org>, Matthew Wilcox <mawilcox@...rosoft.com>, Linux-MM <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>, Kernel Hardening <kernel-hardening@...ts.openwall.com>, cocci@...teme.lip6.fr, Himanshu Jha <himanshujha199640@...il.com> Subject: Re: [PATCH 2/2] mm: Add kvmalloc_ab_c and kvzalloc_struct On 2018-05-01 19:00, Kees Cook wrote: > On Mon, Apr 30, 2018 at 2:29 PM, Rasmus Villemoes > <linux@...musvillemoes.dk> wrote: >> >> 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". 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. > 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? Rasmus
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.