[Openais] [PATCH] V2 Fix debug function in logsys.c (Was: Re: Remove unused functions from logsys.c)

[Fabio M. Di Nitto]

On 6/5/2010 8:31 AM, Andreas Florath wrote:
> Hello!
> 
> Ok - looks that it's needed sometimes during debugging.

Indeed.

> 
> Attached a patch against svn version 2920 which (hopefully)
> does The-Right-Thing and has no buffer overflow.

It is enough to fix the buffer overflow in the old code. This debugging
piece of infrastructure is not performance critical. There is little to
no point to implement all those buffers/strlen optimizations.

> (IMHO also the debug-code should be bug-free ;-) ).

Oh indeed, I absolutely agree. Given the nature of the code itself
(being effectively executed only in a controlled environment and only
once during the debugging process) i didn´t really pay too much
attention to possible security issues. But it´s good somebody spotted
them, regardless of the above conditions.

Cheers
Fabio

> 
> Kind regards
> 
> Andreas Florath
> 
> Signed-off-by: Andreas Florath <gnu4u at flonatel dot org>
> ---
> 
> Index: exec/logsys.c
> ===================================================================
> --- exec/logsys.c	(revision 2920)
> +++ exec/logsys.c	(working copy)
> @@ -217,6 +217,130 @@
>  /* forward declarations */
>  static void logsys_close_logfile(int subsysid);
> 
> +#ifdef LOGSYS_DEBUG
> +struct st_ls2str
> +{
> +	unsigned long  flag;
> +	char const *   name;
> +}
> +	ls2str[] =
> +	{
> +		{ LOGSYS_MODE_OUTPUT_FILE,     "FILE" },
> +		{ LOGSYS_MODE_OUTPUT_STDERR,   "STDERR" },
> +		{ LOGSYS_MODE_OUTPUT_SYSLOG,   "SYSLOG" },
> +		{ LOGSYS_MODE_FORK,            "FORK" },
> +		{ LOGSYS_MODE_THREADED,        "THREADED" }
> +	};
> +
> +/* This is a copy of the strncpy(3) function but it returns the
> + * index to the end of the dest string.
> + * This eliminates the need for additional strlen() calls. */
> +static size_t mstrncpy(char *dest, const char *src, size_t n)
> +{
> +	size_t i;
> +
> +	for (i=0; i<n && src[i]!='\0'; ++i)
> +		dest[i] = src[i];
> +	dest[i] = '\0';
> +
> +	return i;
> +}
> +
> +/* Appends exactly one string to the buffer.
> + * It takes care about the maximal size of buf and the comma
> + * handling between the words.
> + * The function recognizes the fact, that a comma must be inserted, on
> + * the condition whether the used_buf is 0 (->no comma prepended) or
> + * 1 (->comma prepended). */
> +static void decode_mode_append(
> +	unsigned int const mode, char * const buf, size_t const buflen,
> +	struct st_ls2str const * const l2si, size_t * const used_buf)
> +{
> +	if (mode & l2si->flag)
> +	{
> +		if (*used_buf) buf[(*used_buf)++] = ',';
> +		*used_buf += mstrncpy(buf+*used_buf, l2si->name,
> +				      buflen-*used_buf-1);
> +	}
> +}
> +
> +/* Appends all modes with an index less than the given 'maxidx' to the
> + * buffer. */
> +static char * decode_mode_maxidx(
> +	unsigned int mode, char *buf, size_t buflen, int maxidx)
> +{
> +	int idx;
> +	size_t used_buf = 0;
> +	*buf = '\0';
> +
> +	/* buflen-1: the \0 must fit in the buffer */
> +	for (idx=0; idx<maxidx && used_buf<buflen-1; ++idx)
> +		decode_mode_append(
> +			mode, buf, buflen, &ls2str[idx], &used_buf);
> +	return buf;
> +}
> +
> +static char * decode_mode(int subsysid, char *buf, size_t buflen)
> +{
> +	return decode_mode_maxidx(
> +		logsys_loggers[subsysid].mode, buf, buflen,
> +		subsysid == LOGSYS_MAX_SUBSYS_COUNT ? 5 : 3);
> +}
> +
> +static const char *decode_debug(int subsysid)
> +{
> +	if (logsys_loggers[subsysid].debug)
> +		return "on";
> +
> +	return "off";
> +}
> +
> +static const char *decode_status(int subsysid)
> +{
> +	if (!logsys_loggers[subsysid].init_status)
> +		return "INIT_DONE";
> +
> +	return "NEEDS_INIT";
> +}
> +
> +static void dump_subsys_config(int subsysid)
> +{
> +	char modebuf[1024];
> +
> +	fprintf(stderr,
> +		"ID: %d\n"
> +		"subsys: %s\n"
> +		"logfile: %s\n"
> +		"logfile_fp: %p\n"
> +		"mode: %s\n"
> +		"debug: %s\n"
> +		"syslog_fac: %s\n"
> +		"syslog_pri: %s\n"
> +		"logfile_pri: %s\n"
> +		"init_status: %s\n",
> +		subsysid,
> +		logsys_loggers[subsysid].subsys,
> +		logsys_loggers[subsysid].logfile,
> +		logsys_loggers[subsysid].logfile_fp,
> +		decode_mode(subsysid, modebuf, sizeof(modebuf)),
> +		decode_debug(subsysid),
> +		logsys_facility_name_get(logsys_loggers[subsysid].syslog_facility),
> +		logsys_priority_name_get(logsys_loggers[subsysid].syslog_priority),
> +		logsys_priority_name_get(logsys_loggers[subsysid].logfile_priority),
> +		decode_status(subsysid));
> +}
> +
> +static void dump_full_config(void)
> +{
> +	int i;
> +
> +	for (i = 0; i <= LOGSYS_MAX_SUBSYS_COUNT; i++) {
> +		if (strlen(logsys_loggers[i].subsys) > 0)
> +			dump_subsys_config(i);
> +	}
> +}
> +#endif
> +
>  static uint32_t circular_memory_map (void **buf, size_t bytes)
>  {
>  	void *addr_orig;