Keresztfalvi Gabor <[EMAIL PROTECTED]> writes:
> I changed a lot of things in tty.c server.c server_pty.c and client.c.
> It works on my Linux, but I ran into a very wierd problem, which I want to
> trace down. After running n+1 times (n tends to 3) the client without
> restarting the server, sometimes I got bad TERM strings on the server side. I
> traced it down till a parse_string_copy in do_alloc_pty/server.c. In
> parse_string_copy/parse.c I changed the ssh_format to an lsh_string_alloc, and
> a for cycle to put the data to the lsh_string, but I got the same problem.
> Then I started to write out the pointers and length, and got a strange result:
> in the case of the bad copy lsh_string_alloc allocs memory for lsh_string just
> 33 bytes before the _start_ pointer in the function parse_string_copy.
> The length of the string is 5 bytes, so the whole lsh_string structure is 32
> bytes long. If this is the real case then the lsh_string alloc overwrites
> the beginning of the buffer->data. Why does this happen? Whether is it machine
> specific? I successfully reproduced this problem on two (kernels:
> 2.0.35-SuSE, 2.1.130) linux with the original 1999-01-28 snapshot.
> Ideas?
Ah, some functions in channel.c frees packets too early (Ray found
this out some days ago). I suspect this is the same problem. Perhaps
it would be a good idea to report all bugs like this to the list, not
just to me, so that you don't have to struggle with the same problems
several times.
As a work-around, delete the lsh_string_free(packet) calls from
do_global_request, do_channel_open and do_channel_request. (I have a
better fix in my sources, but I haven't got it to compile yet).
Below is an explanation (to go in the next version of doc/NOTES), that
explains how to avoid similar bugs.
: PARSING USING STRUCT SIMPLE_BUFFER
:
: I view the simple_buffer objects as bookkeeping objects only. They
: don't own any storage. They are *always* allocated on the stack, and
: are therefore short-lived and not garbage collected.
:
: The idea is that you initialize a simple_buffer to parse some data,
: and do all the parsing more or less immediately. When parsing is
: done, forget the buffer object, and free the underlying data if
: appropriate.
:
: In most cases, parsing looks like
:
: simple_buffer_init(&buffer, packet->length, packet->data);
: if (parse_*(&buffer, ...) && parse_*(&buffer, ...))
: {
: lsh_string_free(packet);
:
: /* No more uses of buffer, and no more calls to parse_* */
:
: return...
: }
: lsh_string_free(packet);
: return LSH_FAIL;
:
: This usage pattern should be safe.
:
: The do_channel_request() function is an exception, in that it
: delegates some of the parsing to another method. It must therefore
: keep the packet data around until that method has returned.
: Conversely, the called method, CHANNEL_REQUEST, must assume that the
: data associated with its buffer argument will disappear as soon as the
: method returns; any data that is needed later must be copied.
:
: The do_alloc_pty function in server.c observes this; it doesn't use a
: copying method when extracting the mode string, *but* the string is
: used only in the call to tty_decode_term_mode. If the method needed to
: remember the string for later use, it would have to use
: parse_string_copy instead.
:
: In some cases some data from the buffer is needed later, and in those
: cases, it should be copied out of the buffer using parse_string_copy.
: But most parsing extracts integers from the buffer, in various ways,
: and do not need to do any extra copying.
/Niels
