Messages in this thread | | | Date | Sun, 6 Feb 2022 10:49:15 -0800 | From | Kees Cook <> | Subject | Re: [PATCH 2/3] gcc-plugins/stackleak: Exactly match strings instead of prefixes |
| |
On Sun, Feb 06, 2022 at 10:34:11AM -0800, Linus Torvalds wrote: > On Sun, Feb 6, 2022 at 9:45 AM Kees Cook <keescook@chromium.org> wrote: > > > > + return !strncmp(TREE_STRING_POINTER(node), string, length); > > Why is this "strncmp()"? That makes no sense when you've just checked > the exact lengths of both sides. > > You're not comparing strings any more, you've already checked the end > of the string - you are comparing memory contents. > > So make it just do a "memcmp()".
Yeah, good point. I'll change this for v2, pending more feedback.
> > +#define STRING_EQUAL(node, str) string_equal(node, str, strlen(str)) > > .. and please change this name too, since it's not comparing two > strings. The first argument is something else entirely. > > It's checking the node value of a section, give it some name related to that.
Technically, yes. The naming bikeshed here is odd since it's called "STRING" by gcc internals, and it *might* be a "C string", etc etc. I'll rename it...
> I do also get the feeling that the nodes should actually be checked to > be a STRING_CST rather than these blind TREE_VALUE() following things, > but I don't really know the rules for gcc plugin internals very well - > or at all, really.
I'll double-check this, but if it's not a STRING_CST something else has gone very wrong already. But I'm a fan of robustness, so sure. :)
-Kees
-- Kees Cook
| |