[PATCH] About the bug that makes pfinet crash

[Thomas Schwinge]

Hello!

On Fri, Nov 21, 2008 at 12:50:42AM +0100, Samuel Thibault wrote:
> Thomas Schwinge, le Fri 21 Nov 2008 00:08:32 +0100, a écrit :
> > Further printf inspection revealed that skb->len / count was 4110
> > (reproducibly).
> 
> Ah. That's more than a page, it _can_ be implemented in xen by using a
> special kind of request, but I was lazy to implement that

This isn't even needed here, the problem is a different, simpler one.

> believing
> that the IP stack wouldn't overcome the typical 1500B MTU anyway.

It will, because you told it can do so, allowing for a PAGE_SIZE MTU.
But: The MTU is defined to be without the ethernet headers, thus pfinet
can create ethernet frames that are bigger than a page.

Here is a fix.  OK to be committed?

Index: xen/net.c
===================================================================
RCS file: /cvsroot/hurd/gnumach/xen/Attic/net.c,v
retrieving revision 1.1.2.36
diff -u -p -r1.1.2.36 net.c
--- xen/net.c   22 Sep 2008 18:49:56 -0000      1.1.2.36
+++ xen/net.c   22 Nov 2008 21:48:22 -0000
@@ -383,9 +383,10 @@ void hyp_net_init(void) {
                ifp = &nd->ifnet;
                ifp->if_unit = n;
                ifp->if_flags = IFF_UP | IFF_RUNNING;
-               ifp->if_mtu = PAGE_SIZE;
                ifp->if_header_size = 14;
                ifp->if_header_format = HDR_ETHERNET;
+               /* Set to the maximum that we can handle in device_write.  */
+               ifp->if_mtu = PAGE_SIZE - ifp->if_header_size;
                ifp->if_address_size = ADDRESS_SIZE;
                ifp->if_address = (void*) nd->address;
                if_init_queues (ifp);


Additionally, I'd suggest this change to better (?) explain what we're
doing (and align to the other network device_write implementations).

Index: xen/net.c
===================================================================
RCS file: /cvsroot/hurd/gnumach/xen/Attic/net.c,v
retrieving revision 1.1.2.36
diff -u -p -r1.1.2.36 net.c
--- xen/net.c   22 Sep 2008 18:49:56 -0000      1.1.2.36
+++ xen/net.c   22 Nov 2008 21:48:22 -0000
@@ -483,13 +484,15 @@ device_write(void *d, ipc_port_t reply_p
        vm_map_copy_t copy = (vm_map_copy_t) data;
        grant_ref_t gref;
        struct net_data *nd = d;
+       struct ifnet *ifp = &nd->ifnet;
        netif_tx_request_t *req;
        unsigned reqn;
        vm_offset_t offset;
        vm_page_t m;
        vm_size_t size;
 
-       if (count == 0 || count > PAGE_SIZE)
+       if (count < ifp->if_header_size ||
+           count > ifp->if_header_size + ifp->if_mtu)
                return D_INVALID_SIZE;
 
        assert(copy->type == VM_MAP_COPY_PAGE_LIST);


Regards,
 Thomas

signature.asc
Description: Digital signature