Hi, I sent this patch to the list last month, but haven't had any feedback yet. Does anyone have any comments (aside from my poor spelling and grammar)? Am I way off track with this fix or is it ok? Looking forward to your comments. best regards, Anthony

...

...

On 9/14/2009 at 4:50 PM, in message <4ADD7D39.7D71.00E6.0@alliedtelesis.co.nz>, anthony lineham wrote: Hi Bazsi,

I've been investigating a seg fault that occurs when using an include statement in the config file. The error occurs when one of the included files contains an syntax error. The first time syslog-ng is restarted with the bad config, the error is detected and restart is aborted. However, if a second HUP is sent a seg fault occurs.

I had a bit of trouble tracking down the cause and I'm still not entirely sure of the mechanism that leads to the crash but I found a couple of apparent problems, which when corrected prevent it.

1. There is a global variable "include_depth" which normally gets decremented back to zero after successful parsing of included config files. However, if an error is detected it stays at its current value and subsequent restarts increment from that point. This doesn't cause the crash, but given enough restarts would lead to overrun or exhaustion of the "include_stack" array.

2. In the case of unsuccessful config parsing certain bits of memory are dealloced but their pointers that not reset. This seems to be what was causing the seg fault but I've found it a bit hard to pin down.

3. There was an off-by-1 error in the included config deinit loop.

The following patch fixes the problem, but may not necessarily be the best way to do it - particularly issue 1.

Regards, Anthony

--- syslog-ng_3.0.1-63-g41f77f5-old/src/cfg-lex.l 2009-09-14 16:02:09.000000000 +1200 +++ syslog-ng_3.0.1-63-g41f77f5/src/cfg-lex.l 2009-09-14 16:15:30.000000000 +1200 @@ -626,17 +626,24 @@ { gint i;

- for (i = 0; i < include_depth; i++) + for (i = 0; i <= include_depth; i++) { CfgIncludeLevel *level = &include_stack[i];

if (level->current_file) - g_free(level->current_file); + { + g_free(level->current_file); + level->current_file = NULL; + }

g_slist_foreach(level->files, (GFunc) g_free, NULL); g_slist_free(level->files); level->files = NULL; if (level->yybuf) - yy_delete_buffer(level->yybuf); + { + yy_delete_buffer(level->yybuf); + level->yybuf = NULL; + } } + include_depth = 0; }