[Patch] fix buffer truncation when client receives multiple messages

[Paulius Zaleckas]

lgsm_handle doesn't have members you are using...

P.S. Is your tree available to the outside world?

Paulius Zaleckas

andrzej zaborowski wrote:
> 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