In article <pine.neb.4.64.1610260555490.23...@speedy.whooppee.com>,
Paul Goyette <p...@whooppee.com> wrote:
>On Wed, 26 Oct 2016, matthew green wrote:
>
>> i think you're right that the 'cp' manipulation is the problem.
>> snprintf() will return the "desired" size, so upon the first
>> attempted overflow the 'cp' is moved beyond 'ep', and then the
>> next snprintf() gets a negative aka extremely massive value
>> for the buffer length and actual overflow happens here, and ssp
>> detects it.
>>
>> the fix would be to change this:
>>
>> cp += snprintf(cp, ep - cp, ...);
>>
>> into this:
>>
>> len = snprintf(cp, ep - cp, ...);
>> if (len > ep - cp)
>> return;
>> cp += len;
>>
>> which is annoying because there are a lot of the former.
>
>There's only 9 snprintf() calls. I could simply provide a macro:
>
>#define ADD_TEXT(dest, end, format, ...) \
> { \
> int len, max = (end) - (dest); \
>
> len = snprintf((dest), max, (format), __VA_ARGS__); \
> if (len > max) \
> return; \
> (dest) += len; \
> }
>
>
>Then all of the snprintf() calls become simply
>
> ADD_TEXT(cp, ep, <format>, ...);
>
>(Of course, after last use I'd add a #undef ADD_TEXT to clean up...)
Do you really want to stop from attaching the driver because you could
not print it's name? I think that perhaps a variant that returns the
number of characters appended would be more useful. like
if (len >= max)
dest = end;
else
dest += len;
but, I would leave the whole thing alone instead :-)
or write a function that appends and moves the pointer, like
snappendprintf(char **dest, const char *end, fmt, ...)
christos
