linux-kernel - Re: [RFC PATCH V2] core_pattern: fix long parameters was truncated by core

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100802135013.GA5877@redhat.com>
Date:	Mon, 2 Aug 2010 15:50:13 +0200
From:	Oleg Nesterov <oleg@...hat.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>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Neil Horman <nhorman@...driver.com>,
	Roland McGrath <roland@...hat.com>
Subject: Re: [RFC PATCH V2] core_pattern: fix long parameters was truncated
	by core_pattern handler

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.

--
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/