rtsock.c:rt_msg2() appears to be overwriting buffers it does not own

[Brett Lymn <blymn%baesystems.com.au@localhost>]

Folks,

I have been looking at a reported memory corruption problem when pppd
dies.  This was the motivation for the malloclog changes I posted
previously.  With those changes in place all the reported instances of
data being modified on the freelist had a neighbour allocation that
was done in rt_sock.c at line 365.

Inspecting the code there we can see that just before rt_msg2() is
called to calculate the length of the buffer and then the buffer is,
effectively, reallocated if it is too small.  Then rt_msg2() is called
again with the resized buffer.  The way that rt_msg2() is called
results in a memcpy() at line 658 being called, the comments indicate
that it is assumed that the buffer is large enough to hold the
information.  From what I can see there is nothing preventing the
routing information being update between when rt_msg2() is called the
first time to calculate the size of the buffer and the second time
when the allocated buffer is used.

To try and test this theory, I put a few printfs around the suspect
code (see attached patch...) and it produced the following results:

Oct 23 14:15:27 t61 pppd[969]: Terminating on signal 15
Oct 23 14:15:27 t61 pppd[969]: Connect time 0.6 minutes.
Oct 23 14:15:27 t61 pppd[969]: Sent 9936 bytes, received 13408 bytes.
Oct 23 14:15:27 t61 /netbsd: first rt_msg2, len = 224, rtm_msglen = 208
Oct 23 14:15:27 t61 /netbsd: Second rt_msg2, len = 224
Oct 23 14:15:27 t61 /netbsd: len + dlen exceeds *lenp, 400 > 224
Oct 23 14:15:27 t61 /netbsd: len + dlen exceeds *lenp, 416 > 224
Oct 23 14:15:27 t61 /netbsd: len + dlen exceeds *lenp, 432 > 224
Oct 23 14:15:27 t61 /netbsd: len + dlen exceeds *lenp, 448 > 224
....
Oct 23 14:15:27 t61 /netbsd: Data modified on freelist: word 0 of object 0xffff8
000045ba800 size 172 previous type bar (0x80bfcce0 != 0xdeadbeef)
Oct 23 14:15:27 t61 /netbsd: malloc log entry 34694:
....
Oct 23 14:16:06 t61 pppd[1663]: Terminating on signal 15
Oct 23 14:16:06 t61 pppd[1663]: Connect time 0.1 minutes.
Oct 23 14:16:06 t61 pppd[1663]: Sent 65 bytes, received 0 bytes.
Oct 23 14:16:06 t61 /netbsd: first rt_msg2, len = 224, rtm_msglen = 208
Oct 23 14:16:06 t61 /netbsd: Second rt_msg2, len = 224
Oct 23 14:16:06 t61 /netbsd: len + dlen exceeds *lenp, 400 > 224
Oct 23 14:16:06 t61 /netbsd: len + dlen exceeds *lenp, 416 > 224
Oct 23 14:16:06 t61 /netbsd: len + dlen exceeds *lenp, 432 > 224
Oct 23 14:16:06 t61 /netbsd: len + dlen exceeds *lenp, 448 > 224
...
Oct 23 14:16:07 t61 /netbsd: Data modified on freelist: word 0 of object 0xffff8
000045cf100 size 176 previous type bar (0x80bfcce0 != 0xdeadbeef)

I think that this does pretty firmly point the finger at a routing
table update that is exceeding its allocated memory and stomping the
memory after it.  If this is really the case, I am not sure how to fix
it - the whole routing update thing looks like a bit of a MP disaster
as there appears to be no locking about any of the updates.  Ideas?

-- 
Brett Lymn
"Warning:
The information contained in this email and any attached files is
confidential to BAE Systems Australia. If you are not the intended
recipient, any use, disclosure or copying of this email or any
attachments is expressly prohibited.  If you have received this email
in error, please notify us immediately. VIRUS: Every care has been
taken to ensure this email and its attachments are virus free,
however, any loss or damage incurred in using this email is not the
sender's responsibility.  It is your responsibility to ensure virus
checks are completed before installing any data sent in this email to
your computer."

Index: rtsock.c
===================================================================
RCS file: /cvsroot/src/sys/net/rtsock.c,v
retrieving revision 1.111
diff -r1.111 rtsock.c
362a363,364
>                       printf("first rt_msg2, len = %d, rtm_msglen = %d\n",
>                              len, rtm->rtm_msglen);
371c373,374
<                       (void)rt_msg2(rtm->rtm_type, &info, rtm, NULL, 0);
---
>                       printf("Second rt_msg2, len = %d\n", len);
>                       (void)rt_msg2(rtm->rtm_type, &info, rtm, NULL, &len);
659a663,671
>                       if (!second_time) {
>                               if ((len + dlen) > *lenp)
>                                       printf("len + dlen exceeds *lenp, %d > 
> %d\n",
>                                              (len + dlen), *lenp);
>                       } else {
>                               if ((len + dlen) > w->w_tmemsize)
>                                       printf("len + dlen exceeds w_tmemsize, 
> %d > %d\n",
>                                               (len +dlen), w->w_tmemsize);
>                       }