On Mon, Dec 07 2015, Kees Cook <keesc...@chromium.org> wrote: > On Sat, Dec 5, 2015 at 12:38 PM, Rasmus Villemoes > <li...@rasmusvillemoes.dk> wrote: >> I did a search for code doing >> >> s[n]printf(buf, "...", ..., buf, ...) >> >> and found a few instances. They all do it with the format string >> beginning with "%s" and buf being passed as the corresponding parameter >> (obviously to append to the existing string). That works (AFAICT), both >> with the current printf implementation and with the string() >> modification which is now in -mm. It would obviously go horribly wrong >> if anything, even non-specifiers, precede the "%s" in the format >> string. [...] > > If the replacement isn't ugly/complex/error-prone, we should fix it > and find a way to detect the issue. Otherwise, we should leave it and > add it to the printf test cases so we'll notice if it ever regresses.
The usual pattern is to keep the length so far in a variable and do len += snprintf(buf + len, bufsize - len, ...) So this fails if/when overflow is actually possible (we'd end up with a huge positive size being passed, trigger the WARN_ONCE in vsnprintf and get a return value of 0, and that would then repeat). But scnprintf has the property that if you pass a positive bufsize, the return value is strictly less than that; so if one subtracts said return value from the available buffer size, one still has a positive buffer size. (Maybe that invariant should be spelled out somewhere.) IOW, I think that most users of repeated snprintf(buf, bufsize, "%s...", buf, ...) could be replaced by 'int len = 0; ... len += scnprintf(buf + len, bufsize - len, "...", ...);'. Let me go see how that would look. When sprintf is being used I think one can just directly convert to the "len +=" model; one would overflow the buffer if and only if the other would (I think). Rasmus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/