[syslog-ng][PATCH] please test: syslog-ng message mangling fix

[Balazs Scheidler]

On Tue, Aug 20, 2002 at 07:38:21AM -0400, Stephen Frost wrote:
> Balazs and everyone,
> 
>   The first fix which was posted to the list changing the if to be:
> 
>   if (!eol && closure->pos == closure->max_log_line)
> 
>   appeared to work correctly for me.  The second fix posted which
>   changed the if to be:
> 
>   if (!eol && (start_pos == 0 || closure->pos == closure->max_log_line))
> 
>   had the same (broken) behaviour as the unmodified if.  This leads me
>   to conclude that 'closure->pos == closure->max_log_line' is false,
>   '!eol' is true and 'start_pos == 0' is true.  log_msg_size() does not
>   appear to have any effect.  I have increased it to be 2048 and still
>   my logs are split amoung lines.  Message length doesn't appear to
>   matter either as messages which are longer can end up being intact
>   while shorter messages are split.  The one common factor is that all
>   of the messages being split are coming from /proc/kmsg.

hm. as it seems there was a change on the behaviour of /proc/kmsg. The
condition used to be:

-	if (!eol && closure->pos) {
-		/* we don't have a terminating nl nor \0 */

and there was no reports on mangled messages. Maybe 2.4 kernel introduced
a change, and syslog-ng reads kernel message parts with multiple read()
calls.

> 
>   Since there was some concern about UDP messages being broken by this
>   my first thought is to change the if to be:
> 
>   if (!eol && (closure->dgram || closure->pos == closure->max_log_line))
> 
>   This makes the assumption that messages coming from streams such as a
>   TCP connection or /dev/log or a file/pipe will eventually have a
>   terminating newline or \0.  Of course, it keeps the sanity check to
>   not create messages over max_log_line.  I have this working on my
>   setup now but I don't swear that it's perfect, I'd love to hear
>   feedback on it, esp. from Balazs.  These messages being split causes
>   me a real problem on my firewall.

Relying on closure->dgram seems to me a sane decision. start_pos was only a
hack anyway.

Here's an updated patch (still against 1.5.19):

diff -u -r1.34 sources.c
--- sources.c	18 Jul 2002 13:18:02 -0000	1.34
+++ sources.c	21 Aug 2002 12:44:18 -0000
@@ -112,8 +113,11 @@
 		closure->pos = 0;
 		return ST_OK | ST_GOON;
 	}
-	if (!eol && closure->pos) {
-		/* we don't have a terminating nl nor \0 */
+	if (!eol && (closure->dgram || closure->pos == closure->max_log_line)) {
+		/* we don't have a terminating nl nor \0, and our buffer is
+		   full or we are a datagram receiver, when the message is in
+		   its own packet.
+		 */
 		do_handle_line(closure, closure->pos, closure->buffer, salen ? (abstract_addr *) &sabuf : NULL, salen);
 		closure->pos = 0;
 		return ST_OK | ST_GOON;


-- 
Bazsi
PGP info: KeyID 9AF8D0A9 Fingerprint CD27 CFB0 802C 0944 9CFD 804E C82C 8EB1