| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100729130707.GA31154@shamino.rdu.redhat.com>
Date: Thu, 29 Jul 2010 09:31:22 -0400
From: Neil Horman <nhorman@...driver.com>
To: Xiaotian Feng <dfeng@...hat.com>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
Alexander Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>,
Oleg Nesterov <oleg@...hat.com>,
KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Roland McGrath <roland@...hat.com>
Subject: Re: [RFC PATCH] core_pattern: fix long parameters was truncated by
core_pattern handler
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@...hat.com>
> Cc: Alexander Viro <viro@...iv.linux.org.uk>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Oleg Nesterov <oleg@...hat.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> Cc: Neil Horman <nhorman@...driver.com>
> Cc: Roland McGrath <roland@...hat.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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists