Re: mbuf_outbound_finalize() reports packet length is less than mbuf length
Re: mbuf_outbound_finalize() reports packet length is less than mbuf length
- Subject: Re: mbuf_outbound_finalize() reports packet length is less than mbuf length
- From: Brendan Creane <email@hidden>
- Date: Thu, 30 Jul 2009 15:00:18 -0700
Hey All,
My first version of pulling up the ip header wasn't updating the
output filter's mbuf pointer, so pullup would allocate a new mbuf to
copy the data into and free the original. Then further down the chain,
someone else was freeing that mbuf chain again. Sigh. So the code
below mollifies in_delayed_cksum_offset(), and it looks like everyone
else is working as well.
I think Drew suggested parsing the IP header and making sure to pull
in the entire ip header. After looking at in_delayed_cksum_offset(), I
don't think that's necessary -- that routine is only pulling in the IP
header, and it looks as though the checksum is being updated
correctly.
Thanks again for everyone's help (Drew, Kevin, call-out). Now Apple
just needs to fix mbuf_outbound_finalize and all will be well with the
world (bugreporter id: 7095665).
cheers, Brendan
------------------------
errno_t
drv::Filter::pkt_out(ifnet_t interface, protocol_family_t protocol,
mbuf_t *first_buf)
{
mbuf_t m = *first_buf;
// check whether we've seen this packet previously
if (check_mbuf_tag(m, OUTBOUND_DONE))
return KERN_SUCCESS; // pass through the packet
{
/**
* mbuf_outbound_finalize() calls in_delayed_cksum_offset() which
* gets confused when the first mbuf contains only the packet header.
* I've only seen these packets come from the OpenVPN tun/tap interface
* headed out to a physical interface. Pulling in the ip header to the
* first mbuf seems to mollify in_delayed_cksum_offset().
*/
const size_t hdr_len = ifnet_hdrlen(interface);
const size_t first_mbuf_len = mbuf_len(m);
if (first_mbuf_len == hdr_len) {
errno_t err = mbuf_pullup(&m, sizeof(struct iphdr));
if (err != KERN_SUCCESS || m == NULL) {
printf("Filter::pkt_out() - mbuf_pullup err=%d\n", err);
return EJUSTRETURN; // stop processing packet,
don't free it either
} else {
*first_buf = m;
}
}
}
mbuf_outbound_finalize(m, protocol, ifnet_hdrlen(interface));
....
On Tue, Jul 28, 2009 at 4:04 AM, Andrew Gallatin<email@hidden> wrote:
> Brendan Creane wrote:
>
>> The code block that does the pullup is:
>>
>> if (mbuf_len(m) == ifnet_hdrlen(interface)) {
>> err = mbuf_pullup(&m, sizeof(struct iphdr));
>
> Rather than just pulling up 20 bytes, you should dereference
> the IP hdr and pull up the actual size of the hdr (which can be
> longer than sizeof(struct iphdr) if there are options).
>
>> if (err != KERN_SUCCESS) {
>> printf("drv::Filter::pkt_out() - mbuf_pullup err=%d\n",
>> err);
>> return EJUSTRETURN; // "swallow" the packet -
>> mbuf_pullup frees the mbuf on failure
>> }
>> }
>>
>> It looks like mbuf_pullup() correctly sets the "PKTHDR" flag when it
>> allocates a new mbuf and the original mbuf has that bit set. So the
>> reason for the panic isn't clear to me, at this point.
>
> What does the mbuf chain look like at this point?
>
> Drew
>
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Darwin-kernel mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden