[Patch] fix buffer truncation when client receives multiple messages

[andrzej zaborowski]

Hi,

On 04/04/2008, Paulius Zaleckas <paulius.zaleckas at teltonika.lt> wrote:
> I found this bug by running two instances of libgsm-tool -m shell and
> issuing sl=4 command to read more than 10 messages.
>  Cause is that response is bigger than buffer supplied to socket read
> command. When lgsm_handle_packet parses data it finds incomplete data packet
> at the end of this buffer. We receive missing part in next socket read
> command.

Ugh, I'm quite sure I submitted a patch to fix this mid last year.

>  My solution is to copy incomplete part to the beginning of the buffer and
> start filling it from the point where incomplete packet ends.

I used the same idea.  lgsm_handle_packet in my tree looks like this:

int lgsm_handle_packet(struct lgsm_handle *lh, const char *buf, int len)
{
        struct gsmd_msg_hdr *gmh;
        lgsm_msg_handler *handler;
        int rc = 0;

        if (lh->usock_len + len > sizeof(lh->usock_fifo))
                return -ENOMEM;

        memcpy(lh->usock_fifo + lh->usock_len, buf, len);
        lh->usock_len += len;
        gmh = (struct gsmd_msg_hdr *) lh->usock_fifo;
        while (lh->usock_len >= sizeof(*gmh) &&
                        lh->usock_len >= sizeof(*gmh) + gmh->len) {
                if (gmh->msg_type < __NUM_GSMD_MSGS &&
                        (handler = lh->handler[gmh->msg_type]))
                        rc |= handler(lh, gmh);
                else {
                        fprintf(stderr, "unable to handle packet "
                                        "type=%u id=%u\n",
                                        gmh->msg_type, gmh->id);
                        rc |= EINVAL;
                }

                lh->usock_len -= gmh->len + sizeof(*gmh);
                memmove(lh->usock_fifo,
                                lh->usock_fifo + gmh->len + sizeof(*gmh),
                                lh->usock_len);
        }

        return -rc;
}

The usock_* fields are allocated per usock handle (I think this is
cleaner) and the buffer is still limited.

>  This solution is backward compatible.
>
>  I have two more ideas how to solve this:
>  1. Use circular buffer in libgsmd, but this adds more memory usage...
>  2. Do the socket reading inside libgsmd and do smart processing. For
> example read only the size of header check what is the length of the packed
> and dynamically allocate space for it and so on... This would reduce user
> code, improve message handling and reduce memory usage. But in this case we
> will have to change API.

Yup this would be even better, except slightly slower because of all
the malloc()s that would be needed.  This would be the only acceptable
solution for GNU/Hurd for example :)

Cheers
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.