This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Tuesday 03 December 2013 20:39:53 OndÅej BÃlka wrote: > On Sat, Nov 30, 2013 at 02:24:12AM -0500, Mike Frysinger wrote: > > On Tuesday 22 October 2013 19:15:56 OndÅej BÃlka wrote: > > > --- a/debug/stpcpy_chk.c > > > +++ b/debug/stpcpy_chk.c > > > @@ -29,16 +29,7 @@ __stpcpy_chk (dest, src, destlen) > > > > > > const char *src; > > > size_t destlen; > > > > > > { > > > > > > - char *d = dest; > > > - const char *s = src; > > > - > > > - do > > > - { > > > - if (__builtin_expect (destlen-- == 0, 0)) > > > - __chk_fail (); > > > - *d++ = *s; > > > - } > > > - while (*s++ != '\0'); > > > - > > > - return d - 1; > > > + size_t len = strlen (src) + 1; > > > + __memcpy_chk (dest, src, len, destlen); > > > + return dest + len - 1; > > > > > > } > > > > that strlen means you can scan the entire source string twice. seems > > like this overall change might win in some cases, but lose in others. > > > > i think it's safe to assume that the xxx_chk variants are becoming more > > common (i think all the major distros use it by default now) which means > > we need to be concerned with their performance. > > > > that means if an arch wants this to perform, then they should implement > > it which includes the destlen check. > > As this is at most twice slower than nonchecking variant its relatively > reasonable. Worse problem was x64 assembly code which was based on old > implementation and now is around five times slower than this code. > > A one-pass implementation could be done if we add a stpcat/stpncat > functions that returns end of string, these are usefull on their own as it > make easier avoid quadratic loops by concenating strings / letting gcc use > that optimization. > > dest[0] = '\0'; > char *end = stpncat (dest, src, destlen); > if (end - dest == destlen) > __chk_fail (); every once in a while i run into situations where i wish there were more stp variants of existing funcs. maybe worth proposing as GNU extensions ? > > > { > > > - char *s1 = dest; > > > - const char *s2 = src; > > > - char c; > > > - > > > - /* Find the end of the string. */ > > > - do > > > - { > > > - if (__builtin_expect (destlen-- == 0, 0)) > > > - __chk_fail (); > > > - c = *s1++; > > > - } > > > - while (c != '\0'); > > > ... > > > + size_t destlen = strlen (dest); > > > > the dest strlen scan makes sense as we have to do that anyways (find the > > terminating NUL). however, you'll notice that this new version lost a > > check the old one had -- what if dest string lacks a NUL within its > > bounds ? yes, you would catch it below in your single check, but it > > might be too late by that point -- you might have scanned off into > > invalid memory and simply crashed. i think you should use strnlen > > (dest, totalen) instead and check the return value explicitly. > > I did that for performance reasons as segfault is similarly easy to > debug. A strnlen would only mostly work, bound here is not tight, if you > have > > if (c) > x = malloc (100); > else > x = malloc (1000); > > then 1000 will be used as bound. one of the nice things about fortify is that it provides nice abort messages rather than just a segfault. the former are generally taken pretty seriously while the latter are sometimes shrugged off as random flakes. if the performance degradation is significant, i could live with your variant. -mike
Attachment:
signature.asc
Description: This is a digitally signed message part.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |