Messages in this thread | | | Date | Tue, 24 Aug 2010 14:18:32 +0800 | From | Xiaotian Feng <> | Subject | Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler |
| |
On 08/24/2010 05:18 AM, Andrew Morton wrote: > On Fri, 20 Aug 2010 17:35:58 +0800 > Xiaotian Feng<dfeng@redhat.com> wrote: > >> We met a parameter truncated issue, consider following: >>> echo "|/root/core_pattern_pipe_test %p /usr/libexec/blah-blah-blah \ >> %s %c %p %u %g 11 12345678901234567890123456789012345678 %t"> \ >> /proc/sys/kernel/core_pattern >> >> This is okay because the strings is less than CORENAME_MAX_SIZE. >> "cat /proc/sys/kernel/core_pattern" shows the whole string. but >> after we run core_pattern_pipe_test in man page, we found last >> parameter was truncated like below: >> argc[10]=<12807486> >> >> The root cause is core_pattern allows % specifiers, which need to be >> replaced during parse time, but the replace may expand the strings >> to larger than CORENAME_MAX_SIZE. So if the last parameter is % >> specifiers, the replace code is using snprintf(out_ptr, out_end - out_ptr, ...), >> this will write out of corename array. >> >> Changes since v2: >> Introduced generic function cn_printf and make format_corename remember the time >> has been expanded. >> >> Changes since v1: >> This patch allocates corename at runtime, if the replace doesn't have enough >> memory, expand the corename dynamically. > > >> + if (cn->used == cn->size) >> + if (expand_corename(cn)) >> + goto out_fail; >> + >> + out_ptr = cn->corename + cn->used; >> + *out_ptr = *pat_ptr++; >> + cn->used++; > > >> - if (out_ptr == out_end) >> - goto out; >> - *out_ptr++ = '%'; >> + if (cn->used == cn->size) >> + if (expand_corename(cn)) >> + goto out_fail; >> + >> + out_ptr = cn->corename + cn->used; >> + *out_ptr = '%'; >> + cn->used++; > > >> + out_ptr = cn->corename + cn->used; >> + if (cn->used == cn->size) >> + if (expand_corename(cn)) >> + goto out_fail; >> + >> + out_ptr = cn->corename + cn->used; >> *out_ptr = 0; > > Quite a bit of code duplication there. A little helper function which > adds a single char to the output would tidy that up. Yep, this would be much more cleaner ;-) > > However I think that if the % and %% handers are converted to call > cn_printf() then the output is always null-terninated and the third > hunk of code above simply becomes unneeded? You are absolutely right ;-) > > Something like this, although I didn't try very hard. Just a > suggestion to work with ;) > Yep, we just need change a little on your patch
- err = cn_printf(cn, "%%"); + err = cn_printf(cn, "%");
Do you need me to resend a v4 patch?
| |