[syslog-ng] [PATCH 1/7] [persist]: only check the buffer size if no encoding is specified

[Balazs Scheidler]

On Sat, 2011-05-28 at 20:48 +0200, Balazs Scheidler wrote:
> On Sat, 2011-05-28 at 14:30 +0200, Gergely Nagy wrote:
> > Balazs Scheidler <bazsi at balabit.hu> writes:
> > 
> > > Hi,
> > >
> > > Can you please explain when this causes problems and what the problem
> > > is? Does it perhaps need to be backported to earlier versions?
> > 
> > As far as I remember, this caused problems when encoding() was
> > specified for a source (a file source, in this case). In particular, a
> > source file with UCS4 encoding triggered a crash.
> > 
> > As I understand it, if encoding is in place, then the size of
> > self->buffer does not matter all that much, since we'll be using a
> > different buffer. However, due to encoding issues, the raw buffer size
> > might easily be larger than what we have allocated for self->buffer -
> > but that's no big deal in this case, because we'll allocate a large
> > enough buffer in the else branch.
> > 
> > (Note: this is my understanding of the code, this area is a bit hazy for
> > me at the moment)
> > 
> 
> I don't see how this fixes the crash, but I'll see to it if I can
> reproduce the original problem.
> 

I've successfully reproduced the problem. The root cause was that the
error handling path didn't reset some of the state into proper values,
and the crash could happen if any of the error paths were taken.

I've fixed the error handling here:

commit 7a88fb3df322e4b7037fb02475679a52612e2d83
Author: Balazs Scheidler <bazsi at balabit.hu>
Date:   Sun May 29 19:47:19 2011 +0200

    LogProtoBufferedServer: fixed error handling in case the persist-state cannot be restored
    
    In case an error happens while restoring the persist data, be sure to reset
    various state variables to indicate that the Protocol is to be restarted.


And changed the patch slightly to limit raw_buffer_size, since in normal
operation it can't be larger than init_buffer_size (which equals to
log_msg_size()).

This is the integrated patch:
commit 0d8d2d6060c2a99bc5dd8ae399cab703a251259c
Author: Peter Gyorko <gyorkop at balabit.hu>
Date:   Mon May 23 11:56:09 2011 +0200

    [persist]: only check the buffer size if no encoding is specified
    
    If no encoding is specified, we read directly into our buffer,
    thus we need to verify the buffer sizes. If we do use encoding however,
    the raw buffer size cannot be directly compared to our own buffer size,
    we use init_buffer_size instead (which equals to the maximum message size).
    
    Signed-off-by: Viktor Juhasz <jviktor at balabit.hu>
    Signed-off-by: Peter Gyorko <gyorkop at balabit.hu>
    Signed-off-by: Balazs Scheidler <bazsi at balabit.hu>



-- 
Bazsi