* [RFC PATCH] core_pattern: fix long parameters was truncated by core_pattern handler @ 2010-07-29 12:42 Xiaotian Feng 2010-07-29 13:31 ` Neil Horman 0 siblings, 1 reply; 19+ messages in thread From: Xiaotian Feng @ 2010-07-29 12:42 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, Xiaotian Feng, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath 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 %t 11 1234567890123456789012345678901234567890" > \ /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]=<12345678901234567890123456789012345678> 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. This patch expands the size of parsing array, and makes the cursor out_end shift when we replace % specifiers. Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Cc: Neil Horman <nhorman@tuxdriver.com> Cc: Roland McGrath <roland@redhat.com> --- fs/exec.c | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index e19de6a..e67e4b5 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1441,7 +1441,7 @@ EXPORT_SYMBOL(set_binfmt); /* format_corename will inspect the pattern parameter, and output a * name into corename, which must have space for at least - * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. + * CORENAME_MAX_SIZE * 3 bytes because of % specifiers. */ static int format_corename(char *corename, long signr) { @@ -1449,7 +1449,7 @@ static int format_corename(char *corename, long signr) const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); char *out_ptr = corename; - char *const out_end = corename + CORENAME_MAX_SIZE; + char *out_end = corename + CORENAME_MAX_SIZE; int rc; int pid_in_pattern = 0; @@ -1478,6 +1478,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; /* uid */ case 'u': @@ -1486,6 +1487,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; /* gid */ case 'g': @@ -1494,6 +1496,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; /* signal that caused the coredump */ case 's': @@ -1502,6 +1505,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; /* UNIX time of coredump */ case 't': { @@ -1512,6 +1516,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; } /* hostname */ @@ -1523,6 +1528,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; /* executable */ case 'e': @@ -1531,6 +1537,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; /* core limit size */ case 'c': @@ -1539,6 +1546,7 @@ static int format_corename(char *corename, long signr) if (rc > out_end - out_ptr) goto out; out_ptr += rc; + out_end += rc - 2; break; default: break; @@ -1836,7 +1844,7 @@ static int umh_pipe_setup(struct subprocess_info *info) void do_coredump(long signr, int exit_code, struct pt_regs *regs) { struct core_state core_state; - char corename[CORENAME_MAX_SIZE + 1]; + char corename[CORENAME_MAX_SIZE * 3]; struct mm_struct *mm = current->mm; struct linux_binfmt * binfmt; const struct cred *old_cred; -- 1.7.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH] core_pattern: fix long parameters was truncated by core_pattern handler 2010-07-29 12:42 [RFC PATCH] core_pattern: fix long parameters was truncated by core_pattern handler Xiaotian Feng @ 2010-07-29 13:31 ` Neil Horman 2010-08-02 12:23 ` [RFC PATCH V2] " Xiaotian Feng 0 siblings, 1 reply; 19+ messages in thread From: Neil Horman @ 2010-07-29 13:31 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Roland McGrath On Thu, Jul 29, 2010 at 08:42:44PM +0800, Xiaotian Feng 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 %t 11 1234567890123456789012345678901234567890" > \ > /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]=<12345678901234567890123456789012345678> > > 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. > > This patch expands the size of parsing array, and makes the cursor > out_end shift when we replace % specifiers. > > Signed-off-by: Xiaotian Feng <dfeng@redhat.com> > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > Cc: Neil Horman <nhorman@tuxdriver.com> > Cc: Roland McGrath <roland@redhat.com> > --- Thanks for looking at this, its definately a problem. I don't think though, that just extending the size of the corename array to be bigger is really going to solve the problem, you're just running away from it. To really fix it what we should probably do is: 1) Modify the corename argument to format_corename to be char **corename 2) Dynamically allocate an array for corename inside format_corename, of size CORENAME_MAX_SIZE*n, where n is variable holding the maximum number of times we had to increment our allocation size in any previous call to format_corename 3) for each iteration through the while (*pat_ptr) loop in format_corename, check to see if the remaining size can hold the next argument to be parsed. If we're going to overrun, use krealloc to extend the size of the array to another multiple or CORENAME_MAX_SIZE, and increment the variable n in (2) by 1. That should give us a decent heuristic to determine the size of the corename array, so that we never overrun. Regards Neil > fs/exec.c | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index e19de6a..e67e4b5 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1441,7 +1441,7 @@ EXPORT_SYMBOL(set_binfmt); > > /* format_corename will inspect the pattern parameter, and output a > * name into corename, which must have space for at least > - * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. > + * CORENAME_MAX_SIZE * 3 bytes because of % specifiers. > */ > static int format_corename(char *corename, long signr) > { > @@ -1449,7 +1449,7 @@ static int format_corename(char *corename, long signr) > const char *pat_ptr = core_pattern; > int ispipe = (*pat_ptr == '|'); > char *out_ptr = corename; > - char *const out_end = corename + CORENAME_MAX_SIZE; > + char *out_end = corename + CORENAME_MAX_SIZE; > int rc; > int pid_in_pattern = 0; > > @@ -1478,6 +1478,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > /* uid */ > case 'u': > @@ -1486,6 +1487,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > /* gid */ > case 'g': > @@ -1494,6 +1496,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > /* signal that caused the coredump */ > case 's': > @@ -1502,6 +1505,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > /* UNIX time of coredump */ > case 't': { > @@ -1512,6 +1516,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > } > /* hostname */ > @@ -1523,6 +1528,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > /* executable */ > case 'e': > @@ -1531,6 +1537,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > /* core limit size */ > case 'c': > @@ -1539,6 +1546,7 @@ static int format_corename(char *corename, long signr) > if (rc > out_end - out_ptr) > goto out; > out_ptr += rc; > + out_end += rc - 2; > break; > default: > break; > @@ -1836,7 +1844,7 @@ static int umh_pipe_setup(struct subprocess_info *info) > void do_coredump(long signr, int exit_code, struct pt_regs *regs) > { > struct core_state core_state; > - char corename[CORENAME_MAX_SIZE + 1]; > + char corename[CORENAME_MAX_SIZE * 3]; > struct mm_struct *mm = current->mm; > struct linux_binfmt * binfmt; > const struct cred *old_cred; > -- > 1.7.2 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH V2] core_pattern: fix long parameters was truncated by core_pattern handler 2010-07-29 13:31 ` Neil Horman @ 2010-08-02 12:23 ` Xiaotian Feng 2010-08-02 13:50 ` Oleg Nesterov 2010-08-02 14:30 ` Denys Vlasenko 0 siblings, 2 replies; 19+ messages in thread From: Xiaotian Feng @ 2010-08-02 12:23 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, Xiaotian Feng, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath 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. This patch allocates corename at runtime, if the replace doesn't have enough memory, expand the corename dynamically. Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Cc: Neil Horman <nhorman@tuxdriver.com> Cc: Roland McGrath <roland@redhat.com> --- fs/exec.c | 194 ++++++++++++++++++++++++++++++++++++++++++++---------------- 1 files changed, 142 insertions(+), 52 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index e19de6a..d9e1e4f 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1439,26 +1439,56 @@ void set_binfmt(struct linux_binfmt *new) EXPORT_SYMBOL(set_binfmt); +static int expand_corename(char **corename, char **out_end, + char **out_ptr, int *size) +{ + unsigned long ptr_offset; + char *old_corename = *corename; + + (*size)++; + ptr_offset = *out_ptr - *corename; + *corename = krealloc(*corename, *size * CORENAME_MAX_SIZE, + GFP_KERNEL); + + if (*corename == NULL) { + kfree(old_corename); + return -ENOMEM; + } + + *out_end = *corename + *size * CORENAME_MAX_SIZE - 1; + *out_ptr = *corename + ptr_offset; + return 0; +} /* format_corename will inspect the pattern parameter, and output a - * name into corename, which must have space for at least - * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. + * name into corename, which will be allocated at runtime. */ -static int format_corename(char *corename, long signr) +static int format_corename(char **corename, long signr) { const struct cred *cred = current_cred(); const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); - char *out_ptr = corename; - char *const out_end = corename + CORENAME_MAX_SIZE; - int rc; + char *out_ptr, *out_end, *old_corename; + int size = 1; + int rc, ret; int pid_in_pattern = 0; + *corename = kmalloc(CORENAME_MAX_SIZE, GFP_KERNEL); + if (*corename == NULL) + return -ENOMEM; + + old_corename = *corename; + out_ptr = *corename; + out_end = *corename + CORENAME_MAX_SIZE - 1; /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { if (*pat_ptr != '%') { - if (out_ptr == out_end) - goto out; + if (out_ptr == out_end) { + ret = expand_corename(corename, &out_end, + &out_ptr, &size); + if (ret) + return ret; + } *out_ptr++ = *pat_ptr++; } else { switch (*++pat_ptr) { @@ -1466,78 +1496,126 @@ static int format_corename(char *corename, long signr) goto out; /* Double percent, output one percent */ case '%': - if (out_ptr == out_end) - goto out; *out_ptr++ = '%'; break; /* pid */ case 'p': pid_in_pattern = 1; - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%d", + task_tgid_vnr(current)); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%d", + task_tgid_vnr(current)); out_ptr += rc; break; /* uid */ case 'u': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->uid); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%d", cred->uid); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%d", cred->uid); out_ptr += rc; break; /* gid */ case 'g': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->gid); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%d", cred->gid); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%d", cred->gid); out_ptr += rc; break; /* signal that caused the coredump */ case 's': - rc = snprintf(out_ptr, out_end - out_ptr, - "%ld", signr); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%ld", signr); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret == -ENOMEM) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%ld", signr); out_ptr += rc; break; /* UNIX time of coredump */ case 't': { struct timeval tv; do_gettimeofday(&tv); - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", tv.tv_sec); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%lu", tv.tv_sec); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%lu", + tv.tv_sec); out_ptr += rc; break; } /* hostname */ case 'h': down_read(&uts_sem); - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", utsname()->nodename); + rc = snprintf(NULL, 0, "%s", + utsname()->nodename); + up_read(&uts_sem); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret) + return ret; + } + down_read(&uts_sem); + rc = snprintf(out_ptr, rc + 1, "%s", + utsname()->nodename); up_read(&uts_sem); - if (rc > out_end - out_ptr) - goto out; out_ptr += rc; break; /* executable */ case 'e': - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", current->comm); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%s", current->comm); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%s", + current->comm); out_ptr += rc; break; /* core limit size */ case 'c': - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", rlimit(RLIMIT_CORE)); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, "%lu", + rlimit(RLIMIT_CORE)); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, + &out_end, + &out_ptr, &size); + if (ret == -ENOMEM) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%lu", + rlimit(RLIMIT_CORE)); out_ptr += rc; break; default: @@ -1552,10 +1630,14 @@ static int format_corename(char *corename, long signr) * and core_uses_pid is set, then .%pid will be appended to * the filename. Do not do this for piped commands. */ if (!ispipe && !pid_in_pattern && core_uses_pid) { - rc = snprintf(out_ptr, out_end - out_ptr, - ".%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; + rc = snprintf(NULL, 0, ".%d", task_tgid_vnr(current)); + if (rc > out_end - out_ptr) { + ret = expand_corename(corename, &out_end, &out_ptr, + &size); + if (ret == -ENOMEM) + return ret; + } + rc = snprintf(out_ptr, rc + 1, "%d", task_tgid_vnr(current)); out_ptr += rc; } out: @@ -1836,7 +1918,7 @@ static int umh_pipe_setup(struct subprocess_info *info) void do_coredump(long signr, int exit_code, struct pt_regs *regs) { struct core_state core_state; - char corename[CORENAME_MAX_SIZE + 1]; + char *corename; struct mm_struct *mm = current->mm; struct linux_binfmt * binfmt; const struct cred *old_cred; @@ -1895,11 +1977,17 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) * lock_kernel() because format_corename() is controlled by sysctl, which * uses lock_kernel() */ - lock_kernel(); - ispipe = format_corename(corename, signr); + lock_kernel(); + ispipe = format_corename(&corename, signr); unlock_kernel(); - if (ispipe) { + if (ispipe == -ENOMEM) { + printk(KERN_WARNING "format_corename failed\n"); + printk(KERN_WARNING "Aborting core\n"); + goto fail_corename; + } + + if (ispipe) { int dump_count; char **helper_argv; @@ -1946,10 +2034,10 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) NULL, &cprm); argv_free(helper_argv); if (retval) { - printk(KERN_INFO "Core dump to %s pipe failed\n", + printk(KERN_INFO "Core dump to %s pipe failed\n", corename); goto close_fail; - } + } } else { struct inode *inode; @@ -1998,6 +2086,8 @@ fail_dropcount: if (ispipe) atomic_dec(&core_dump_count); fail_unlock: + kfree(corename); +fail_corename: coredump_finish(mm); revert_creds(old_cred); fail_creds: -- 1.7.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH V2] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-02 12:23 ` [RFC PATCH V2] " Xiaotian Feng @ 2010-08-02 13:50 ` Oleg Nesterov 2010-08-03 10:59 ` Neil Horman 2010-08-02 14:30 ` Denys Vlasenko 1 sibling, 1 reply; 19+ messages in thread From: Oleg Nesterov @ 2010-08-02 13:50 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, KOSAKI Motohiro, Neil Horman, Roland McGrath On 08/02, Xiaotian Feng wrote: > > @@ -1466,78 +1496,126 @@ static int format_corename(char *corename, long signr) > goto out; > /* Double percent, output one percent */ > case '%': > - if (out_ptr == out_end) > - goto out; > *out_ptr++ = '%'; Hmm. Not sure I understand why we do not need to check the space here. > - rc = snprintf(out_ptr, out_end - out_ptr, > - "%d", task_tgid_vnr(current)); > - if (rc > out_end - out_ptr) > - goto out; > + rc = snprintf(NULL, 0, "%d", > + task_tgid_vnr(current)); > + if (rc > out_end - out_ptr) { > + ret = expand_corename(corename, > + &out_end, > + &out_ptr, &size); > + if (ret) > + return ret; > + } > + rc = snprintf(out_ptr, rc + 1, "%d", > + task_tgid_vnr(current)); Probably it makes sense to factor out this code? Roughly, something like: struct core_name { char *corename; int len, free; }; static bool cn_printf(struct core_name *cn, const char *fmt, ...) { char *cur; int need; va_list ap; retry: cur = cn->corename + (cn->len - cn->free); need = vsnprintf(cur, cn->free, fmt, ap); if (likely(need < free)) { free -= need; return true; } increase ->len, realloc ->corename or return false; goto retry; } Then format_corename() can just do if (!cn_printf(&cn, ...)) return -ENOMEM; consistently. Also. Not sure this really makes sense, but if we ever need to expand the string, perhaps it makes sense to remeber this fact so that the next time we start with len > CORENAME_MAX_SIZE. In any case, I think this needs a separate patch. Oleg. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH V2] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-02 13:50 ` Oleg Nesterov @ 2010-08-03 10:59 ` Neil Horman 2010-08-20 9:22 ` [RFC PATCH v3] " Xiaotian Feng 2010-08-20 9:35 ` Xiaotian Feng 0 siblings, 2 replies; 19+ messages in thread From: Neil Horman @ 2010-08-03 10:59 UTC (permalink / raw) To: Oleg Nesterov Cc: Xiaotian Feng, linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, KOSAKI Motohiro, Roland McGrath On Mon, Aug 02, 2010 at 03:50:13PM +0200, Oleg Nesterov wrote: > > Also. Not sure this really makes sense, but if we ever need to expand the > string, perhaps it makes sense to remeber this fact so that the next time > we start with len > CORENAME_MAX_SIZE. In any case, I think this needs a > separate patch. > > Oleg. Yes, this is what I alluded to in my initial reply. We need to keep an atomic value to track the maximum number of times we've called expand_corename to help prevent us having to call it repeatedly on every crash. Something like this: expand_corename(**corename, **out_end, **out_ptr) { ... *corename = krealloc(*corename, CORENAME_MAX_SIZE* atomic_inc_return(call_count), GFP_KERNEL); ... } do_coredump(...) { ... corename = kmalloc(CORENAME_MAX_SIZE * atomic_read(call_count), GFP_KERNEL); ... } That will train the path to allocate a sensible size after the first crash, and avoid lots of calls to krealloc in the pessimal case Neil ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-03 10:59 ` Neil Horman @ 2010-08-20 9:22 ` Xiaotian Feng 2010-08-20 9:35 ` Xiaotian Feng 2010-08-20 9:35 ` Xiaotian Feng 1 sibling, 1 reply; 19+ messages in thread From: Xiaotian Feng @ 2010-08-20 9:22 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, Xiaotian Feng, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath 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. Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Cc: Neil Horman <nhorman@tuxdriver.com> Cc: Roland McGrath <roland@redhat.com> --- fs/exec.c | 181 +++++++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 121 insertions(+), 60 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 2d94552..e2fe568 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -65,6 +65,12 @@ char core_pattern[CORENAME_MAX_SIZE] = "core"; unsigned int core_pipe_limit; int suid_dumpable = 0; +struct core_name { + char *corename; + int used, size; +}; +static atomic_t call_count = ATOMIC_INIT(1); + /* The maximal length of core_pattern is also specified in sysctl.c */ static LIST_HEAD(formats); @@ -1440,106 +1446,147 @@ void set_binfmt(struct linux_binfmt *new) EXPORT_SYMBOL(set_binfmt); +static int expand_corename(struct core_name *cn) +{ + char *old_corename = cn->corename; + + cn->size = CORENAME_MAX_SIZE * atomic_inc_return(&call_count); + cn->corename = krealloc(old_corename, cn->size, GFP_KERNEL); + + if (!cn->corename) { + kfree(old_corename); + return -ENOMEM; + } + + return 0; +} + +static int cn_printf(struct core_name *cn, const char *fmt, ...) +{ + char *cur; + int need; + int ret; + va_list arg; + + cur = cn->corename + cn->used; + + va_start(arg, fmt); + need = vsnprintf(NULL, 0, fmt, arg); + va_end(arg); + + if (likely(need < cn->size - cn->used)) + goto out_printf; + + ret = expand_corename(cn); + if (ret) + goto expand_fail; + +out_printf: + va_start(arg, fmt); + vsnprintf(cur, need + 1, fmt, arg); + va_end(arg); + cn->used += need; + return 0; + +expand_fail: + va_end(arg); + return ret; +} + /* format_corename will inspect the pattern parameter, and output a * name into corename, which must have space for at least * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. */ -static int format_corename(char *corename, long signr) +static int format_corename(struct core_name *cn, long signr) { const struct cred *cred = current_cred(); const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); - char *out_ptr = corename; - char *const out_end = corename + CORENAME_MAX_SIZE; - int rc; + char *out_ptr; int pid_in_pattern = 0; + cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); + cn->corename = kmalloc(cn->size, GFP_KERNEL); + cn->used = 0; + + if (!cn->corename) + goto out_fail; + /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { if (*pat_ptr != '%') { - if (out_ptr == out_end) - goto out; - *out_ptr++ = *pat_ptr++; + if (cn->used == cn->size) + if (expand_corename(cn)) + goto out_fail; + + out_ptr = cn->corename + cn->used; + *out_ptr = *pat_ptr++; + cn->used++; } else { switch (*++pat_ptr) { case 0: goto out; /* Double percent, output one percent */ case '%': - 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++; break; /* pid */ case 'p': pid_in_pattern = 1; - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%d", + task_tgid_vnr(current))) + goto out_fail; break; /* uid */ case 'u': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->uid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%d", cred->uid)) + goto out_fail; break; /* gid */ case 'g': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->gid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%d", cred->gid)) + goto out_fail; break; /* signal that caused the coredump */ case 's': - rc = snprintf(out_ptr, out_end - out_ptr, - "%ld", signr); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%ld", signr)) + goto out_fail; break; /* UNIX time of coredump */ case 't': { struct timeval tv; do_gettimeofday(&tv); - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", tv.tv_sec); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%lu", tv.tv_sec)) + goto out_fail; break; } /* hostname */ case 'h': down_read(&uts_sem); - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", utsname()->nodename); + if (cn_printf(cn, "%s", + utsname()->nodename)) { + up_read(&uts_sem); + goto out_fail; + } up_read(&uts_sem); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; break; /* executable */ case 'e': - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", current->comm); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%s", current->comm)) + goto out_fail; break; /* core limit size */ case 'c': - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", rlimit(RLIMIT_CORE)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%lu", + rlimit(RLIMIT_CORE))) + goto out_fail; break; default: break; @@ -1553,15 +1600,21 @@ static int format_corename(char *corename, long signr) * and core_uses_pid is set, then .%pid will be appended to * the filename. Do not do this for piped commands. */ if (!ispipe && !pid_in_pattern && core_uses_pid) { - rc = snprintf(out_ptr, out_end - out_ptr, - ".%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, ".%d", task_tgid_vnr(current))) + goto out_fail; } out: + 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; return ispipe; + +out_fail: + return -ENOMEM; } static int zap_process(struct task_struct *start, int exit_code) @@ -1837,7 +1890,7 @@ static int umh_pipe_setup(struct subprocess_info *info) void do_coredump(long signr, int exit_code, struct pt_regs *regs) { struct core_state core_state; - char corename[CORENAME_MAX_SIZE + 1]; + struct core_name cn; struct mm_struct *mm = current->mm; struct linux_binfmt * binfmt; const struct cred *old_cred; @@ -1892,7 +1945,13 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) */ clear_thread_flag(TIF_SIGPENDING); - ispipe = format_corename(corename, signr); + ispipe = format_corename(&cn, signr); + + if (ispipe == -ENOMEM) { + printk(KERN_WARNING "format_corename failed\n"); + printk(KERN_WARNING "Aborting core\n"); + goto fail_corename; + } if (ispipe) { int dump_count; @@ -1929,7 +1988,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) goto fail_dropcount; } - helper_argv = argv_split(GFP_KERNEL, corename+1, NULL); + helper_argv = argv_split(GFP_KERNEL, cn.corename+1, NULL); if (!helper_argv) { printk(KERN_WARNING "%s failed to allocate memory\n", __func__); @@ -1942,7 +2001,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) argv_free(helper_argv); if (retval) { printk(KERN_INFO "Core dump to %s pipe failed\n", - corename); + cn.corename); goto close_fail; } } else { @@ -1951,7 +2010,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) if (cprm.limit < binfmt->min_coredump) goto fail_unlock; - cprm.file = filp_open(corename, + cprm.file = filp_open(cn.corename, O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag, 0600); if (IS_ERR(cprm.file)) @@ -1993,6 +2052,8 @@ fail_dropcount: if (ispipe) atomic_dec(&core_dump_count); fail_unlock: + kfree(cn.corename); +fail_corename: coredump_finish(mm); revert_creds(old_cred); fail_creds: -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-20 9:22 ` [RFC PATCH v3] " Xiaotian Feng @ 2010-08-20 9:35 ` Xiaotian Feng 0 siblings, 0 replies; 19+ messages in thread From: Xiaotian Feng @ 2010-08-20 9:35 UTC (permalink / raw) To: linux-fsdevel Cc: Xiaotian Feng, linux-kernel, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath On 08/20/2010 05:22 PM, Xiaotian Feng 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. > > Signed-off-by: Xiaotian Feng<dfeng@redhat.com> > Cc: Alexander Viro<viro@zeniv.linux.org.uk> > Cc: Andrew Morton<akpm@linux-foundation.org> > Cc: Oleg Nesterov<oleg@redhat.com> > Cc: KOSAKI Motohiro<kosaki.motohiro@jp.fujitsu.com> > Cc: Neil Horman<nhorman@tuxdriver.com> > Cc: Roland McGrath<roland@redhat.com> > --- > fs/exec.c | 181 +++++++++++++++++++++++++++++++++++++++++-------------------- > 1 files changed, 121 insertions(+), 60 deletions(-) > > diff --git a/fs/exec.c b/fs/exec.c > index 2d94552..e2fe568 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -65,6 +65,12 @@ char core_pattern[CORENAME_MAX_SIZE] = "core"; > unsigned int core_pipe_limit; > int suid_dumpable = 0; > > +struct core_name { > + char *corename; > + int used, size; > +}; > +static atomic_t call_count = ATOMIC_INIT(1); > + > /* The maximal length of core_pattern is also specified in sysctl.c */ > > static LIST_HEAD(formats); > @@ -1440,106 +1446,147 @@ void set_binfmt(struct linux_binfmt *new) > > EXPORT_SYMBOL(set_binfmt); > > +static int expand_corename(struct core_name *cn) > +{ > + char *old_corename = cn->corename; > + > + cn->size = CORENAME_MAX_SIZE * atomic_inc_return(&call_count); > + cn->corename = krealloc(old_corename, cn->size, GFP_KERNEL); > + > + if (!cn->corename) { > + kfree(old_corename); > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static int cn_printf(struct core_name *cn, const char *fmt, ...) > +{ > + char *cur; > + int need; > + int ret; > + va_list arg; > + > + cur = cn->corename + cn->used; > + > + va_start(arg, fmt); > + need = vsnprintf(NULL, 0, fmt, arg); > + va_end(arg); > + > + if (likely(need< cn->size - cn->used)) > + goto out_printf; > + > + ret = expand_corename(cn); > + if (ret) > + goto expand_fail; > + > +out_printf: > + va_start(arg, fmt); > + vsnprintf(cur, need + 1, fmt, arg); > + va_end(arg); > + cn->used += need; > + return 0; > + > +expand_fail: > + va_end(arg); oops, this line should be removed, please ignore this mail, I'll send an updated patch. Thanks Xiaotian ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-03 10:59 ` Neil Horman 2010-08-20 9:22 ` [RFC PATCH v3] " Xiaotian Feng @ 2010-08-20 9:35 ` Xiaotian Feng 2010-08-23 11:07 ` Neil Horman 2010-08-23 21:18 ` Andrew Morton 1 sibling, 2 replies; 19+ messages in thread From: Xiaotian Feng @ 2010-08-20 9:35 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, Xiaotian Feng, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath 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. Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Cc: Neil Horman <nhorman@tuxdriver.com> Cc: Roland McGrath <roland@redhat.com> --- fs/exec.c | 180 ++++++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 120 insertions(+), 60 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 2d94552..d21c8c0 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -65,6 +65,12 @@ char core_pattern[CORENAME_MAX_SIZE] = "core"; unsigned int core_pipe_limit; int suid_dumpable = 0; +struct core_name { + char *corename; + int used, size; +}; +static atomic_t call_count = ATOMIC_INIT(1); + /* The maximal length of core_pattern is also specified in sysctl.c */ static LIST_HEAD(formats); @@ -1440,106 +1446,146 @@ void set_binfmt(struct linux_binfmt *new) EXPORT_SYMBOL(set_binfmt); +static int expand_corename(struct core_name *cn) +{ + char *old_corename = cn->corename; + + cn->size = CORENAME_MAX_SIZE * atomic_inc_return(&call_count); + cn->corename = krealloc(old_corename, cn->size, GFP_KERNEL); + + if (!cn->corename) { + kfree(old_corename); + return -ENOMEM; + } + + return 0; +} + +static int cn_printf(struct core_name *cn, const char *fmt, ...) +{ + char *cur; + int need; + int ret; + va_list arg; + + cur = cn->corename + cn->used; + + va_start(arg, fmt); + need = vsnprintf(NULL, 0, fmt, arg); + va_end(arg); + + if (likely(need < cn->size - cn->used)) + goto out_printf; + + ret = expand_corename(cn); + if (ret) + goto expand_fail; + +out_printf: + va_start(arg, fmt); + vsnprintf(cur, need + 1, fmt, arg); + va_end(arg); + cn->used += need; + return 0; + +expand_fail: + return ret; +} + /* format_corename will inspect the pattern parameter, and output a * name into corename, which must have space for at least * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. */ -static int format_corename(char *corename, long signr) +static int format_corename(struct core_name *cn, long signr) { const struct cred *cred = current_cred(); const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); - char *out_ptr = corename; - char *const out_end = corename + CORENAME_MAX_SIZE; - int rc; + char *out_ptr; int pid_in_pattern = 0; + cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); + cn->corename = kmalloc(cn->size, GFP_KERNEL); + cn->used = 0; + + if (!cn->corename) + goto out_fail; + /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { if (*pat_ptr != '%') { - if (out_ptr == out_end) - goto out; - *out_ptr++ = *pat_ptr++; + if (cn->used == cn->size) + if (expand_corename(cn)) + goto out_fail; + + out_ptr = cn->corename + cn->used; + *out_ptr = *pat_ptr++; + cn->used++; } else { switch (*++pat_ptr) { case 0: goto out; /* Double percent, output one percent */ case '%': - 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++; break; /* pid */ case 'p': pid_in_pattern = 1; - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%d", + task_tgid_vnr(current))) + goto out_fail; break; /* uid */ case 'u': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->uid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%d", cred->uid)) + goto out_fail; break; /* gid */ case 'g': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->gid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%d", cred->gid)) + goto out_fail; break; /* signal that caused the coredump */ case 's': - rc = snprintf(out_ptr, out_end - out_ptr, - "%ld", signr); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%ld", signr)) + goto out_fail; break; /* UNIX time of coredump */ case 't': { struct timeval tv; do_gettimeofday(&tv); - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", tv.tv_sec); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%lu", tv.tv_sec)) + goto out_fail; break; } /* hostname */ case 'h': down_read(&uts_sem); - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", utsname()->nodename); + if (cn_printf(cn, "%s", + utsname()->nodename)) { + up_read(&uts_sem); + goto out_fail; + } up_read(&uts_sem); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; break; /* executable */ case 'e': - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", current->comm); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%s", current->comm)) + goto out_fail; break; /* core limit size */ case 'c': - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", rlimit(RLIMIT_CORE)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, "%lu", + rlimit(RLIMIT_CORE))) + goto out_fail; break; default: break; @@ -1553,15 +1599,21 @@ static int format_corename(char *corename, long signr) * and core_uses_pid is set, then .%pid will be appended to * the filename. Do not do this for piped commands. */ if (!ispipe && !pid_in_pattern && core_uses_pid) { - rc = snprintf(out_ptr, out_end - out_ptr, - ".%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + if (cn_printf(cn, ".%d", task_tgid_vnr(current))) + goto out_fail; } out: + 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; return ispipe; + +out_fail: + return -ENOMEM; } static int zap_process(struct task_struct *start, int exit_code) @@ -1837,7 +1889,7 @@ static int umh_pipe_setup(struct subprocess_info *info) void do_coredump(long signr, int exit_code, struct pt_regs *regs) { struct core_state core_state; - char corename[CORENAME_MAX_SIZE + 1]; + struct core_name cn; struct mm_struct *mm = current->mm; struct linux_binfmt * binfmt; const struct cred *old_cred; @@ -1892,7 +1944,13 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) */ clear_thread_flag(TIF_SIGPENDING); - ispipe = format_corename(corename, signr); + ispipe = format_corename(&cn, signr); + + if (ispipe == -ENOMEM) { + printk(KERN_WARNING "format_corename failed\n"); + printk(KERN_WARNING "Aborting core\n"); + goto fail_corename; + } if (ispipe) { int dump_count; @@ -1929,7 +1987,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) goto fail_dropcount; } - helper_argv = argv_split(GFP_KERNEL, corename+1, NULL); + helper_argv = argv_split(GFP_KERNEL, cn.corename+1, NULL); if (!helper_argv) { printk(KERN_WARNING "%s failed to allocate memory\n", __func__); @@ -1942,7 +2000,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) argv_free(helper_argv); if (retval) { printk(KERN_INFO "Core dump to %s pipe failed\n", - corename); + cn.corename); goto close_fail; } } else { @@ -1951,7 +2009,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) if (cprm.limit < binfmt->min_coredump) goto fail_unlock; - cprm.file = filp_open(corename, + cprm.file = filp_open(cn.corename, O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag, 0600); if (IS_ERR(cprm.file)) @@ -1993,6 +2051,8 @@ fail_dropcount: if (ispipe) atomic_dec(&core_dump_count); fail_unlock: + kfree(cn.corename); +fail_corename: coredump_finish(mm); revert_creds(old_cred); fail_creds: -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-20 9:35 ` Xiaotian Feng @ 2010-08-23 11:07 ` Neil Horman 2010-08-23 23:02 ` KOSAKI Motohiro 2010-08-23 21:18 ` Andrew Morton 1 sibling, 1 reply; 19+ messages in thread From: Neil Horman @ 2010-08-23 11:07 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Roland McGrath On Fri, Aug 20, 2010 at 05:35:58PM +0800, Xiaotian Feng 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. > > Signed-off-by: Xiaotian Feng <dfeng@redhat.com> > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > Cc: Neil Horman <nhorman@tuxdriver.com> > Cc: Roland McGrath <roland@redhat.com> > --- > fs/exec.c | 180 ++++++++++++++++++++++++++++++++++++++++-------------------- > 1 files changed, 120 insertions(+), 60 deletions(-) > This looks alot cleaner. Thanks! Reviewed-by: Neil Horman <nhorman@tuxdriver.com> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-23 11:07 ` Neil Horman @ 2010-08-23 23:02 ` KOSAKI Motohiro 0 siblings, 0 replies; 19+ messages in thread From: KOSAKI Motohiro @ 2010-08-23 23:02 UTC (permalink / raw) To: Neil Horman Cc: kosaki.motohiro, Xiaotian Feng, linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, Oleg Nesterov, Roland McGrath > On Fri, Aug 20, 2010 at 05:35:58PM +0800, Xiaotian Feng 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. > > > > Signed-off-by: Xiaotian Feng <dfeng@redhat.com> > > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Oleg Nesterov <oleg@redhat.com> > > Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> > > Cc: Neil Horman <nhorman@tuxdriver.com> > > Cc: Roland McGrath <roland@redhat.com> > > --- > > fs/exec.c | 180 ++++++++++++++++++++++++++++++++++++++++-------------------- > > 1 files changed, 120 insertions(+), 60 deletions(-) > > > This looks alot cleaner. Thanks! > Reviewed-by: Neil Horman <nhorman@tuxdriver.com> > Unfortunatelly, I have no time to review deeply this one. but I see my test works. Also, I'd like to say I like this one. Thanks. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-20 9:35 ` Xiaotian Feng 2010-08-23 11:07 ` Neil Horman @ 2010-08-23 21:18 ` Andrew Morton 2010-08-24 6:18 ` Xiaotian Feng 2010-08-24 9:42 ` [PATCH v4] " Xiaotian Feng 1 sibling, 2 replies; 19+ messages in thread From: Andrew Morton @ 2010-08-23 21:18 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath 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. 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? Something like this, although I didn't try very hard. Just a suggestion to work with ;) fs/exec.c | 63 ++++++++++++++-------------------------------------- 1 file changed, 17 insertions(+), 46 deletions(-) diff -puN fs/exec.c~core_pattern-fix-long-parameters-was-truncated-by-core_pattern-handler-fix fs/exec.c --- a/fs/exec.c~core_pattern-fix-long-parameters-was-truncated-by-core_pattern-handler-fix +++ a/fs/exec.c @@ -1503,89 +1503,66 @@ static int format_corename(struct core_n int ispipe = (*pat_ptr == '|'); char *out_ptr; int pid_in_pattern = 0; + int err = 0; cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); cn->corename = kmalloc(cn->size, GFP_KERNEL); cn->used = 0; if (!cn->corename) - goto out_fail; + return -ENOMEM; /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { if (*pat_ptr != '%') { - if (cn->used == cn->size) - if (expand_corename(cn)) - goto out_fail; - - out_ptr = cn->corename + cn->used; - *out_ptr = *pat_ptr++; - cn->used++; + err = cn_printf(cn, "%c", *pat_ptr++); } else { switch (*++pat_ptr) { case 0: goto out; /* Double percent, output one percent */ case '%': - if (cn->used == cn->size) - if (expand_corename(cn)) - goto out_fail; - - out_ptr = cn->corename + cn->used; - *out_ptr = '%'; - cn->used++; + err = cn_printf(cn, "%%"); break; /* pid */ case 'p': pid_in_pattern = 1; - if (cn_printf(cn, "%d", - task_tgid_vnr(current))) - goto out_fail; + err = cn_printf(cn, "%d", + task_tgid_vnr(current)); break; /* uid */ case 'u': - if (cn_printf(cn, "%d", cred->uid)) - goto out_fail; + err = cn_printf(cn, "%d", cred->uid); break; /* gid */ case 'g': - if (cn_printf(cn, "%d", cred->gid)) - goto out_fail; + err = cn_printf(cn, "%d", cred->gid); break; /* signal that caused the coredump */ case 's': - if (cn_printf(cn, "%ld", signr)) - goto out_fail; + err = cn_printf(cn, "%ld", signr); break; /* UNIX time of coredump */ case 't': { struct timeval tv; do_gettimeofday(&tv); - if (cn_printf(cn, "%lu", tv.tv_sec)) - goto out_fail; + err = cn_printf(cn, "%lu", tv.tv_sec); break; } /* hostname */ case 'h': down_read(&uts_sem); - if (cn_printf(cn, "%s", - utsname()->nodename)) { - up_read(&uts_sem); - goto out_fail; - } + err = cn_printf(cn, "%s", utsname()->nodename); up_read(&uts_sem); break; /* executable */ case 'e': - if (cn_printf(cn, "%s", current->comm)) - goto out_fail; + err = cn_printf(cn, "%s", current->comm); break; /* core limit size */ case 'c': - if (cn_printf(cn, "%lu", - rlimit(RLIMIT_CORE))) - goto out_fail; + err = cn_printf(cn, "%lu", rlimit(RLIMIT_CORE)); break; default: break; @@ -1593,6 +1570,10 @@ static int format_corename(struct core_n ++pat_ptr; } } + + if (err) + return err; + /* Backward compatibility with core_uses_pid: * * If core_pattern does not include a %p (as is the default) @@ -1603,17 +1584,7 @@ static int format_corename(struct core_n goto out_fail; } out: - 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; return ispipe; - -out_fail: - return -ENOMEM; } static int zap_process(struct task_struct *start, int exit_code) _ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-23 21:18 ` Andrew Morton @ 2010-08-24 6:18 ` Xiaotian Feng 2010-08-24 6:28 ` Andrew Morton 2010-08-24 9:42 ` [PATCH v4] " Xiaotian Feng 1 sibling, 1 reply; 19+ messages in thread From: Xiaotian Feng @ 2010-08-24 6:18 UTC (permalink / raw) To: Andrew Morton Cc: linux-fsdevel, linux-kernel, Alexander Viro, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath 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? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v3] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-24 6:18 ` Xiaotian Feng @ 2010-08-24 6:28 ` Andrew Morton 0 siblings, 0 replies; 19+ messages in thread From: Andrew Morton @ 2010-08-24 6:28 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath On Tue, 24 Aug 2010 14:18:32 +0800 Xiaotian Feng <dfeng@redhat.com> wrote: > > > > 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, "%"); whoa. Closer than I usually achieve. > Do you need me to resend a v4 patch? Yes please - something which was nicely tested ;) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-23 21:18 ` Andrew Morton 2010-08-24 6:18 ` Xiaotian Feng @ 2010-08-24 9:42 ` Xiaotian Feng 2010-08-24 22:47 ` Andrew Morton 1 sibling, 1 reply; 19+ messages in thread From: Xiaotian Feng @ 2010-08-24 9:42 UTC (permalink / raw) To: akpm, linux-fsdevel Cc: nhorman, linux-kernel, Xiaotian Feng, Alexander Viro, Oleg Nesterov, KOSAKI Motohiro, Roland McGrath 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 v3: make handling of single char also uses cn_printf, suggested by Andrew Morton. Changes since v2: Introduced generic function cn_printf and make format_corename remember the time has been expanded, suggested by Olg Nesterov and Neil Horman. Changes since v1: This patch allocates corename at runtime, if the replace doesn't have enough memory, expand the corename dynamically, suggested by Neil Horman. I've tested with some core_pattern strings, it works fine now. Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Reviewed-by: Neil Horman <nhorman@tuxdriver.com> Cc: Roland McGrath <roland@redhat.com> --- fs/exec.c | 158 +++++++++++++++++++++++++++++++++++++----------------------- 1 files changed, 97 insertions(+), 61 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 2d94552..0151923 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -65,6 +65,12 @@ char core_pattern[CORENAME_MAX_SIZE] = "core"; unsigned int core_pipe_limit; int suid_dumpable = 0; +struct core_name { + char *corename; + int used, size; +}; +static atomic_t call_count = ATOMIC_INIT(1); + /* The maximal length of core_pattern is also specified in sysctl.c */ static LIST_HEAD(formats); @@ -1440,127 +1446,149 @@ void set_binfmt(struct linux_binfmt *new) EXPORT_SYMBOL(set_binfmt); +static int expand_corename(struct core_name *cn) +{ + char *old_corename = cn->corename; + + cn->size = CORENAME_MAX_SIZE * atomic_inc_return(&call_count); + cn->corename = krealloc(old_corename, cn->size, GFP_KERNEL); + + if (!cn->corename) { + kfree(old_corename); + return -ENOMEM; + } + + return 0; +} + +static int cn_printf(struct core_name *cn, const char *fmt, ...) +{ + char *cur; + int need; + int ret; + va_list arg; + + va_start(arg, fmt); + need = vsnprintf(NULL, 0, fmt, arg); + va_end(arg); + + if (likely(need < cn->size - cn->used)) + goto out_printf; + + ret = expand_corename(cn); + if (ret) + goto expand_fail; + +out_printf: + cur = cn->corename + cn->used; + va_start(arg, fmt); + vsnprintf(cur, need + 1, fmt, arg); + va_end(arg); + cn->used += need; + return 0; + +expand_fail: + return ret; +} + /* format_corename will inspect the pattern parameter, and output a * name into corename, which must have space for at least * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. */ -static int format_corename(char *corename, long signr) +static int format_corename(struct core_name *cn, long signr) { const struct cred *cred = current_cred(); const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); - char *out_ptr = corename; - char *const out_end = corename + CORENAME_MAX_SIZE; - int rc; int pid_in_pattern = 0; + int err = 0; + + cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); + cn->corename = kmalloc(cn->size, GFP_KERNEL); + cn->used = 0; + + if (!cn->corename) + return -ENOMEM; /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { if (*pat_ptr != '%') { - if (out_ptr == out_end) - goto out; - *out_ptr++ = *pat_ptr++; + err = cn_printf(cn, "%c", *pat_ptr++); } else { switch (*++pat_ptr) { + /* single % at the end, drop that */ case 0: + err = cn_printf(cn, "%c", '\0'); + if (err) + break; goto out; /* Double percent, output one percent */ case '%': - if (out_ptr == out_end) - goto out; - *out_ptr++ = '%'; + err = cn_printf(cn, "%c", '%'); break; /* pid */ case 'p': pid_in_pattern = 1; - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%d", + task_tgid_vnr(current)); break; /* uid */ case 'u': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->uid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%d", cred->uid); break; /* gid */ case 'g': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->gid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%d", cred->gid); break; /* signal that caused the coredump */ case 's': - rc = snprintf(out_ptr, out_end - out_ptr, - "%ld", signr); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%ld", signr); break; /* UNIX time of coredump */ case 't': { struct timeval tv; do_gettimeofday(&tv); - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", tv.tv_sec); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%lu", tv.tv_sec); break; } /* hostname */ case 'h': down_read(&uts_sem); - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", utsname()->nodename); + err = cn_printf(cn, "%s", + utsname()->nodename); up_read(&uts_sem); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; break; /* executable */ case 'e': - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", current->comm); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%s", current->comm); break; /* core limit size */ case 'c': - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", rlimit(RLIMIT_CORE)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%lu", + rlimit(RLIMIT_CORE)); break; default: break; } ++pat_ptr; } + + if (err) + return err; } + /* Backward compatibility with core_uses_pid: * * If core_pattern does not include a %p (as is the default) * and core_uses_pid is set, then .%pid will be appended to * the filename. Do not do this for piped commands. */ if (!ispipe && !pid_in_pattern && core_uses_pid) { - rc = snprintf(out_ptr, out_end - out_ptr, - ".%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, ".%d", task_tgid_vnr(current)); + if (err) + return err; } out: - *out_ptr = 0; return ispipe; } @@ -1837,7 +1865,7 @@ static int umh_pipe_setup(struct subprocess_info *info) void do_coredump(long signr, int exit_code, struct pt_regs *regs) { struct core_state core_state; - char corename[CORENAME_MAX_SIZE + 1]; + struct core_name cn; struct mm_struct *mm = current->mm; struct linux_binfmt * binfmt; const struct cred *old_cred; @@ -1892,7 +1920,13 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) */ clear_thread_flag(TIF_SIGPENDING); - ispipe = format_corename(corename, signr); + ispipe = format_corename(&cn, signr); + + if (ispipe == -ENOMEM) { + printk(KERN_WARNING "format_corename failed\n"); + printk(KERN_WARNING "Aborting core\n"); + goto fail_corename; + } if (ispipe) { int dump_count; @@ -1929,7 +1963,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) goto fail_dropcount; } - helper_argv = argv_split(GFP_KERNEL, corename+1, NULL); + helper_argv = argv_split(GFP_KERNEL, cn.corename+1, NULL); if (!helper_argv) { printk(KERN_WARNING "%s failed to allocate memory\n", __func__); @@ -1942,7 +1976,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) argv_free(helper_argv); if (retval) { printk(KERN_INFO "Core dump to %s pipe failed\n", - corename); + cn.corename); goto close_fail; } } else { @@ -1951,7 +1985,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) if (cprm.limit < binfmt->min_coredump) goto fail_unlock; - cprm.file = filp_open(corename, + cprm.file = filp_open(cn.corename, O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag, 0600); if (IS_ERR(cprm.file)) @@ -1993,6 +2027,8 @@ fail_dropcount: if (ispipe) atomic_dec(&core_dump_count); fail_unlock: + kfree(cn.corename); +fail_corename: coredump_finish(mm); revert_creds(old_cred); fail_creds: -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-24 9:42 ` [PATCH v4] " Xiaotian Feng @ 2010-08-24 22:47 ` Andrew Morton 2010-08-25 1:58 ` Xiaotian Feng 2010-08-25 2:17 ` [PATCH v5] " Xiaotian Feng 0 siblings, 2 replies; 19+ messages in thread From: Andrew Morton @ 2010-08-24 22:47 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, nhorman, linux-kernel, Alexander Viro, Oleg Nesterov, KOSAKI Motohiro, Roland McGrath On Tue, 24 Aug 2010 17:42:46 +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 v3: > make handling of single char also uses cn_printf, suggested by Andrew Morton. > > Changes since v2: > Introduced generic function cn_printf and make format_corename remember the time > has been expanded, suggested by Olg Nesterov and Neil Horman. > > Changes since v1: > This patch allocates corename at runtime, if the replace doesn't have enough > memory, expand the corename dynamically, suggested by Neil Horman. > > I've tested with some core_pattern strings, it works fine now. cool, thanks. > > ... > > -static int format_corename(char *corename, long signr) > +static int format_corename(struct core_name *cn, long signr) > { > const struct cred *cred = current_cred(); > const char *pat_ptr = core_pattern; > int ispipe = (*pat_ptr == '|'); > - char *out_ptr = corename; > - char *const out_end = corename + CORENAME_MAX_SIZE; > - int rc; > int pid_in_pattern = 0; > + int err = 0; > + > + cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); > + cn->corename = kmalloc(cn->size, GFP_KERNEL); > + cn->used = 0; > + > + if (!cn->corename) > + return -ENOMEM; > > /* Repeat as long as we have more pattern to process and more output > space */ > while (*pat_ptr) { > if (*pat_ptr != '%') { > - if (out_ptr == out_end) > - goto out; > - *out_ptr++ = *pat_ptr++; > + err = cn_printf(cn, "%c", *pat_ptr++); > } else { > switch (*++pat_ptr) { > + /* single % at the end, drop that */ > case 0: > + err = cn_printf(cn, "%c", '\0'); Confused. Doesn't this bit just add another \0 to the end of an already-null-terminated string? And then make cn->used get out of sync with strlen(cn->corename)? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-24 22:47 ` Andrew Morton @ 2010-08-25 1:58 ` Xiaotian Feng 2010-08-25 2:17 ` [PATCH v5] " Xiaotian Feng 1 sibling, 0 replies; 19+ messages in thread From: Xiaotian Feng @ 2010-08-25 1:58 UTC (permalink / raw) To: Andrew Morton Cc: linux-fsdevel, nhorman, linux-kernel, Alexander Viro, Oleg Nesterov, KOSAKI Motohiro, Roland McGrath On 08/25/2010 06:47 AM, Andrew Morton wrote: > On Tue, 24 Aug 2010 17:42:46 +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 v3: >> make handling of single char also uses cn_printf, suggested by Andrew Morton. >> >> Changes since v2: >> Introduced generic function cn_printf and make format_corename remember the time >> has been expanded, suggested by Olg Nesterov and Neil Horman. >> >> Changes since v1: >> This patch allocates corename at runtime, if the replace doesn't have enough >> memory, expand the corename dynamically, suggested by Neil Horman. >> >> I've tested with some core_pattern strings, it works fine now. > > cool, thanks. > >> >> ... >> >> -static int format_corename(char *corename, long signr) >> +static int format_corename(struct core_name *cn, long signr) >> { >> const struct cred *cred = current_cred(); >> const char *pat_ptr = core_pattern; >> int ispipe = (*pat_ptr == '|'); >> - char *out_ptr = corename; >> - char *const out_end = corename + CORENAME_MAX_SIZE; >> - int rc; >> int pid_in_pattern = 0; >> + int err = 0; >> + >> + cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); >> + cn->corename = kmalloc(cn->size, GFP_KERNEL); >> + cn->used = 0; >> + >> + if (!cn->corename) >> + return -ENOMEM; >> >> /* Repeat as long as we have more pattern to process and more output >> space */ >> while (*pat_ptr) { >> if (*pat_ptr != '%') { >> - if (out_ptr == out_end) >> - goto out; >> - *out_ptr++ = *pat_ptr++; >> + err = cn_printf(cn, "%c", *pat_ptr++); >> } else { >> switch (*++pat_ptr) { >> + /* single % at the end, drop that */ >> case 0: >> + err = cn_printf(cn, "%c", '\0'); > > Confused. Doesn't this bit just add another \0 to the end of an > already-null-terminated string? And then make cn->used get out of sync > with strlen(cn->corename)? > Good catch, I just realized the return value of vsnprintf is not including the trailing '\0', will follow an updated v5 patch. Thanks Andrew. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-24 22:47 ` Andrew Morton 2010-08-25 1:58 ` Xiaotian Feng @ 2010-08-25 2:17 ` Xiaotian Feng 1 sibling, 0 replies; 19+ messages in thread From: Xiaotian Feng @ 2010-08-25 2:17 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, nhorman, Xiaotian Feng, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Roland McGrath 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 v4: cn_printf needs to take trailing '\0' into account. Then we don't need to copy extra '\0' while parsing pat_ptr. Changes since v3: make handling of single char also uses cn_printf, suggested by Andrew Morton. Changes since v2: Introduced generic function cn_printf and make format_corename remember the time has been expanded, suggested by Olg Nesterov and Neil Horman. Changes since v1: This patch allocates corename at runtime, if the replace doesn't have enough memory, expand the corename dynamically, suggested by Neil Horman. I've tested with some core_pattern strings, it works fine now. Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Reviewed-by: Neil Horman <nhorman@tuxdriver.com> Cc: Roland McGrath <roland@redhat.com> --- fs/exec.c | 155 +++++++++++++++++++++++++++++++++++++----------------------- 1 files changed, 95 insertions(+), 60 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 2d94552..ca3550c 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -65,6 +65,12 @@ char core_pattern[CORENAME_MAX_SIZE] = "core"; unsigned int core_pipe_limit; int suid_dumpable = 0; +struct core_name { + char *corename; + int used, size; +}; +static atomic_t call_count = ATOMIC_INIT(1); + /* The maximal length of core_pattern is also specified in sysctl.c */ static LIST_HEAD(formats); @@ -1440,127 +1446,148 @@ void set_binfmt(struct linux_binfmt *new) EXPORT_SYMBOL(set_binfmt); +static int expand_corename(struct core_name *cn) +{ + char *old_corename = cn->corename; + + cn->size = CORENAME_MAX_SIZE * atomic_inc_return(&call_count); + cn->corename = krealloc(old_corename, cn->size, GFP_KERNEL); + + if (!cn->corename) { + kfree(old_corename); + return -ENOMEM; + } + + return 0; +} + +static int cn_printf(struct core_name *cn, const char *fmt, ...) +{ + char *cur; + int need; + int ret; + va_list arg; + + va_start(arg, fmt); + need = vsnprintf(NULL, 0, fmt, arg); + va_end(arg); + + if (likely(need < cn->size - cn->used -1)) + goto out_printf; + + ret = expand_corename(cn); + if (ret) + goto expand_fail; + +out_printf: + cur = cn->corename + cn->used; + va_start(arg, fmt); + vsnprintf(cur, need + 1, fmt, arg); + va_end(arg); + cn->used += need; + return 0; + +expand_fail: + return ret; +} + /* format_corename will inspect the pattern parameter, and output a * name into corename, which must have space for at least * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator. */ -static int format_corename(char *corename, long signr) +static int format_corename(struct core_name *cn, long signr) { const struct cred *cred = current_cred(); const char *pat_ptr = core_pattern; int ispipe = (*pat_ptr == '|'); - char *out_ptr = corename; - char *const out_end = corename + CORENAME_MAX_SIZE; - int rc; int pid_in_pattern = 0; + int err = 0; + + cn->size = CORENAME_MAX_SIZE * atomic_read(&call_count); + cn->corename = kmalloc(cn->size, GFP_KERNEL); + cn->used = 0; + + if (!cn->corename) + return -ENOMEM; /* Repeat as long as we have more pattern to process and more output space */ while (*pat_ptr) { if (*pat_ptr != '%') { - if (out_ptr == out_end) + if (*pat_ptr == 0) goto out; - *out_ptr++ = *pat_ptr++; + err = cn_printf(cn, "%c", *pat_ptr++); } else { switch (*++pat_ptr) { + /* single % at the end, drop that */ case 0: goto out; /* Double percent, output one percent */ case '%': - if (out_ptr == out_end) - goto out; - *out_ptr++ = '%'; + err = cn_printf(cn, "%c", '%'); break; /* pid */ case 'p': pid_in_pattern = 1; - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%d", + task_tgid_vnr(current)); break; /* uid */ case 'u': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->uid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%d", cred->uid); break; /* gid */ case 'g': - rc = snprintf(out_ptr, out_end - out_ptr, - "%d", cred->gid); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%d", cred->gid); break; /* signal that caused the coredump */ case 's': - rc = snprintf(out_ptr, out_end - out_ptr, - "%ld", signr); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%ld", signr); break; /* UNIX time of coredump */ case 't': { struct timeval tv; do_gettimeofday(&tv); - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", tv.tv_sec); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%lu", tv.tv_sec); break; } /* hostname */ case 'h': down_read(&uts_sem); - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", utsname()->nodename); + err = cn_printf(cn, "%s", + utsname()->nodename); up_read(&uts_sem); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; break; /* executable */ case 'e': - rc = snprintf(out_ptr, out_end - out_ptr, - "%s", current->comm); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%s", current->comm); break; /* core limit size */ case 'c': - rc = snprintf(out_ptr, out_end - out_ptr, - "%lu", rlimit(RLIMIT_CORE)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, "%lu", + rlimit(RLIMIT_CORE)); break; default: break; } ++pat_ptr; } + + if (err) + return err; } + /* Backward compatibility with core_uses_pid: * * If core_pattern does not include a %p (as is the default) * and core_uses_pid is set, then .%pid will be appended to * the filename. Do not do this for piped commands. */ if (!ispipe && !pid_in_pattern && core_uses_pid) { - rc = snprintf(out_ptr, out_end - out_ptr, - ".%d", task_tgid_vnr(current)); - if (rc > out_end - out_ptr) - goto out; - out_ptr += rc; + err = cn_printf(cn, ".%d", task_tgid_vnr(current)); + if (err) + return err; } out: - *out_ptr = 0; return ispipe; } @@ -1837,7 +1864,7 @@ static int umh_pipe_setup(struct subprocess_info *info) void do_coredump(long signr, int exit_code, struct pt_regs *regs) { struct core_state core_state; - char corename[CORENAME_MAX_SIZE + 1]; + struct core_name cn; struct mm_struct *mm = current->mm; struct linux_binfmt * binfmt; const struct cred *old_cred; @@ -1892,7 +1919,13 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) */ clear_thread_flag(TIF_SIGPENDING); - ispipe = format_corename(corename, signr); + ispipe = format_corename(&cn, signr); + + if (ispipe == -ENOMEM) { + printk(KERN_WARNING "format_corename failed\n"); + printk(KERN_WARNING "Aborting core\n"); + goto fail_corename; + } if (ispipe) { int dump_count; @@ -1929,7 +1962,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) goto fail_dropcount; } - helper_argv = argv_split(GFP_KERNEL, corename+1, NULL); + helper_argv = argv_split(GFP_KERNEL, cn.corename+1, NULL); if (!helper_argv) { printk(KERN_WARNING "%s failed to allocate memory\n", __func__); @@ -1942,7 +1975,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) argv_free(helper_argv); if (retval) { printk(KERN_INFO "Core dump to %s pipe failed\n", - corename); + cn.corename); goto close_fail; } } else { @@ -1951,7 +1984,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs) if (cprm.limit < binfmt->min_coredump) goto fail_unlock; - cprm.file = filp_open(corename, + cprm.file = filp_open(cn.corename, O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag, 0600); if (IS_ERR(cprm.file)) @@ -1993,6 +2026,8 @@ fail_dropcount: if (ispipe) atomic_dec(&core_dump_count); fail_unlock: + kfree(cn.corename); +fail_corename: coredump_finish(mm); revert_creds(old_cred); fail_creds: -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH V2] core_pattern: fix long parameters was truncated by core_pattern handler 2010-08-02 12:23 ` [RFC PATCH V2] " Xiaotian Feng @ 2010-08-02 14:30 ` Denys Vlasenko 2010-08-02 14:30 ` Denys Vlasenko 1 sibling, 0 replies; 19+ messages in thread From: Denys Vlasenko @ 2010-08-02 14:30 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath > + rc = snprintf(NULL, 0, "%s", current->comm); rc = strlen(current->comm) would be smaller -- vda ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH V2] core_pattern: fix long parameters was truncated by core_pattern handler @ 2010-08-02 14:30 ` Denys Vlasenko 0 siblings, 0 replies; 19+ messages in thread From: Denys Vlasenko @ 2010-08-02 14:30 UTC (permalink / raw) To: Xiaotian Feng Cc: linux-fsdevel, linux-kernel, Alexander Viro, Andrew Morton, Oleg Nesterov, KOSAKI Motohiro, Neil Horman, Roland McGrath > + rc = snprintf(NULL, 0, "%s", current->comm); rc = strlen(current->comm) would be smaller -- vda -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-08-25 2:18 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-29 12:42 [RFC PATCH] core_pattern: fix long parameters was truncated by core_pattern handler Xiaotian Feng 2010-07-29 13:31 ` Neil Horman 2010-08-02 12:23 ` [RFC PATCH V2] " Xiaotian Feng 2010-08-02 13:50 ` Oleg Nesterov 2010-08-03 10:59 ` Neil Horman 2010-08-20 9:22 ` [RFC PATCH v3] " Xiaotian Feng 2010-08-20 9:35 ` Xiaotian Feng 2010-08-20 9:35 ` Xiaotian Feng 2010-08-23 11:07 ` Neil Horman 2010-08-23 23:02 ` KOSAKI Motohiro 2010-08-23 21:18 ` Andrew Morton 2010-08-24 6:18 ` Xiaotian Feng 2010-08-24 6:28 ` Andrew Morton 2010-08-24 9:42 ` [PATCH v4] " Xiaotian Feng 2010-08-24 22:47 ` Andrew Morton 2010-08-25 1:58 ` Xiaotian Feng 2010-08-25 2:17 ` [PATCH v5] " Xiaotian Feng 2010-08-02 14:30 ` [RFC PATCH V2] " Denys Vlasenko 2010-08-02 14:30 ` Denys Vlasenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.