Messages in this thread | | | Date | Sun, 24 Aug 2014 13:40:58 -0700 | From | Dave Hansen <> | Subject | Re: [PATCH] [v3] warn on performance-impacting configs aka. TAINT_PERFORMANCE |
| |
On 08/24/2014 07:49 AM, Ingo Molnar wrote: >>>> > >> + buf_left = buf_len; >>>> > >> + for (i = 0; i < ARRAY_SIZE(perfomance_killing_configs); i++) { >>>> > >> + buf_written += snprintf(buf + buf_written, buf_left, >>>> > >> + "%s%s\n", config_prefix, >>>> > >> + perfomance_killing_configs[i]); >>>> > >> + buf_left = buf_len - buf_written; ... >>> > > Also, do you want to check buf_left and break out early from >>> > > the loop if it goes non-positive? >> > >> > You're slowly inflating my patch for no practical gain. :) > AFAICS it's a potential memory corruption and security bug, > should the array ever grow large enough to overflow the passed > in buffer size.
Let's say there is 1 "buf_left" and I attempt a 100-byte snprintf(). Won't snprintf() return 1, and buf_written will then equal buf_len? buf_left=0 at that point, and will get passed in to the next snprintf() as the buffer length. I'm expecting snprintf() to just return 0 when it gets a 0 for its 'size'.
Exhausting the buffer will, at worst, mean a bunch of useless calls to snprintf() that do nothing, but I don't think it will run over the end of the buffer.
Or am I missing something?
| |