Sending built-by-hand packet and kernel panic.

Sending built-by-hand packet and kernel panic.

[Emmanuel Guiton]

Hi!

Can any experienced network programmer give a quick look at the 
following? I try to send a TCP packet built by hand. It works, but it 
also leads to a kernel panic if a second packet is to be sent. As I'm 
not an experienced kernel programmer, I guess I'm doing some stupid 
thing that can be easily solved.

Basically, my code just fills a sk_buff structure and sends it with NF_HOOK.

Thanks for any help,
           Emmanuel



        /* skb is allocated 56 bytes = TCP message with no data 
(detailed hereafter)
         * + 2 extra bytes before the ethernet header (see hereafter)
         */
    skb = alloc_skb(56, GFP_ATOMIC);
    skb_reserve(skb, 2); /* for 16-bit alignment*/
        /* ethernet header is 14 byte long */
    eth = (__u8 *) skb_put(skb, 14);
        /* ip header is 20 byte long */
    iph = (struct iphdr *)skb_put(skb, sizeof(struct iphdr));
        /* tcp header (no options) is 20 byte long */
    tcph = (struct tcphdr *)skb_put(skb, sizeof(struct tcphdr));

    /* skb->dst AND skb->dev (the latter is set by the former) */
    if (ip_route_output(&rt, ip_dst, 0, 0, 0) != 0)
    {
        printk("ip_route_output failed.\n");
        return -1;
    }
    skb->dst = (struct rt_entry *) rt; /* A trick from ip_route_input.c */
    skb->dev = skb->dst->dev;
   
    /* Socket allocation. */
    if (sock_create(PF_INET, SOCK_RAW, IPPROTO_RAW, &sending_socket) < 0)
    {   
        printk("Error socket creation.\n");
        sock_release(sending_socket);
        return -1;
    }
    sk = kmalloc(sizeof(struct sock), GFP_KERNEL);
    memcpy(&(sending_socket->sk), sk, sizeof(struct sock));
    sock_release(sending_socket);
    if (sk == NULL)
    {
        printk("Error: sk == NULL\n");
        return -1;
    }
        /* Now, set the sock field. */
    skb->sk = sk;
   
    /* TRANSPORT HEADER: TCP */
        /* here, TCP header data is filled in tcph. */
    skb->h.th = tcph;
    /* NETWORK HEADER: IP */
        /* here, IP header data is filled in iph*/
    skb->nh.iph = iph;
    /* LINK HEADER: ETHERNET */
        /* here, ethernet header data is filled in iph*/
    skb->mac.ethernet = ((u8 *)iph) - 14;

    /* Fix me:
     * Scheduling priority put at max, choose more correct value.
     */
    skb->priority = 15;
    /* To choose right pkt_type when receiving a packet.
     * We're not receivng anything, but I set the value like the guys
     * in pktgen.c did, there should be a reason for that.
     */
    skb->protocol = __constant_htons(ETH_P_IP);
    
    /* TIMESTAMP */
        /* last, so it's closest to sending time. */
    do_gettimeofday(&skb->stamp);
    
printk("Going to send initialized skb! ...\n");
//    if (NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dev, 
output_maybe_reroute) < 0)
    NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dev, 
ip_finish_output) < 0

Freeing skbuff (was: Re: Sending built-by-hand packet and kernel panic.)

[Emmanuel Guiton]

Hi!

Thanks a lot for pointing out these problems. I had completely missed them.
However, my overall problem is not solved. As far as my investigations 
led me, my sk_buff structure is never released after having been sent on 
the wire. So I guess I need an explicit destructor function in my 
sk_buff as the following is present in the definition of struct sk_buff:
void         (*destructor)(struct sk_buff *);    /* Destruct function    
    */

Well, until now what I tried lead to even more quicker kernel panics. If 
anyone has a good advice, I'd appreciate a lot.

'Thank you again,

         Emmanuel


Duncan Sands wrote:

>Hi Emmanuel, I don't know anything about network programming
>but I did notice a few strange things:
>
>  
>
>>        /* skb is allocated 56 bytes = TCP message with no data
>>(detailed hereafter)
>>         * + 2 extra bytes before the ethernet header (see hereafter)
>>         */
>>    skb = alloc_skb(56, GFP_ATOMIC);
>>    
>>
>
>Why is this GFP_ATOMIC when you use GFP_KERNEL later on?
>
>  
>
>>    skb_reserve(skb, 2); /* for 16-bit alignment*/
>>        /* ethernet header is 14 byte long */
>>    eth = (__u8 *) skb_put(skb, 14);
>>        /* ip header is 20 byte long */
>>    iph = (struct iphdr *)skb_put(skb, sizeof(struct iphdr));
>>        /* tcp header (no options) is 20 byte long */
>>    tcph = (struct tcphdr *)skb_put(skb, sizeof(struct tcphdr));
>>
>>    /* skb->dst AND skb->dev (the latter is set by the former) */
>>    if (ip_route_output(&rt, ip_dst, 0, 0, 0) != 0)
>>    {
>>        printk("ip_route_output failed.\n");
>>        return -1;
>>    }
>>    skb->dst = (struct rt_entry *) rt; /* A trick from ip_route_input.c */
>>    skb->dev = skb->dst->dev;
>>
>>    /* Socket allocation. */
>>    if (sock_create(PF_INET, SOCK_RAW, IPPROTO_RAW, &sending_socket) < 0)
>>    {
>>        printk("Error socket creation.\n");
>>        sock_release(sending_socket);
>>        return -1;
>>    }
>>    sk = kmalloc(sizeof(struct sock), GFP_KERNEL);
>>    memcpy(&(sending_socket->sk), sk, sizeof(struct sock));
>>    
>>
>
>Here you are copying the (uninitialized) sk into sending_socket->sk.
>I guess you got the arguments to memcpy the wrong way round.
>
>  
>
>>    sock_release(sending_socket);
>>    
>>
>
>Maybe this drops reference counts to various objects, in which
>case it is wrong to reference them via sk.
>
>  
>
>>    if (sk == NULL)
>>    {
>>        printk("Error: sk == NULL\n");
>>        return -1;
>>    }
>>        /* Now, set the sock field. */
>>    skb->sk = sk;
>>
>>    /* TRANSPORT HEADER: TCP */
>>        /* here, TCP header data is filled in tcph. */
>>    skb->h.th = tcph;
>>    /* NETWORK HEADER: IP */
>>        /* here, IP header data is filled in iph*/
>>    skb->nh.iph = iph;
>>    /* LINK HEADER: ETHERNET */
>>        /* here, ethernet header data is filled in iph*/
>>    skb->mac.ethernet = ((u8 *)iph) - 14;
>>
>>    /* Fix me:
>>     * Scheduling priority put at max, choose more correct value.
>>     */
>>    skb->priority = 15;
>>    /* To choose right pkt_type when receiving a packet.
>>     * We're not receivng anything, but I set the value like the guys
>>     * in pktgen.c did, there should be a reason for that.
>>     */
>>    skb->protocol = __constant_htons(ETH_P_IP);
>>
>>    /* TIMESTAMP */
>>        /* last, so it's closest to sending time. */
>>    do_gettimeofday(&skb->stamp);
>>
>>printk("Going to send initialized skb! ...\n");
>>//    if (NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dev,
>>output_maybe_reroute) < 0)
>>    NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dev,
>>ip_finish_output) < 0
>>    
>>
>
>All the best,
>
>Duncan.
>  
>

Re: Freeing skbuff (was: Re: Sending built-by-hand packet and kernel panic.)

[Muli Ben-Yehuda]

[-- Attachment #1: Type: text/plain, Size: 1400 bytes --]

On Mon, Feb 02, 2004 at 07:51:47PM +0200, Emmanuel Guiton wrote:

> However, my overall problem is not solved. As far as my investigations 
> led me, my sk_buff structure is never released after having been sent on 
> the wire. So I guess I need an explicit destructor function in my 
> sk_buff as the following is present in the definition of struct sk_buff:
> void         (*destructor)(struct sk_buff *);    /* Destruct function  */

Note that depending on what you're doing, you might not be able to use
the destructor, because the upper layers use it without regards to
whether it was set before. To the best of my understanding, the rules
for the destructor say that it is free for the use of whatever layer
owns the skbuff at the moment. There are three ways around it - the
first and obvious is to avoid relying on the destructor. The second is
that you can use skb_clone() to get your own copy of the headers and
the destructor (but that doesn't really help you because how does the
layer that ends up freeing the skb know to use your version of the
headers?) and the third is to use this patch,
http://www.mulix.org/patches/skb-destructor-chaining-A2-2.6.1, to 
allow more than destructor per skb. 

Hope this helps, 
Muli 
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/

"the nucleus of linux oscillates my world" - gccbot@#offtopic


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Freeing skbuff (was: Re: Sending built-by-hand packet and kernel panic.)

[Emmanuel Guiton]

Hi!

Well, my investigations led me to precise a bit more my problem. There 
is no problem with NF_HOOK as it returns 0 and the packet is sent on the 
wire.
Upper layers are not a problem neither, because I bypass them totally. 
However, handling myself the dtructor function seems to be definitely a 
must do.

Concerning what I noticed that was wrong in my code, there was this 
trick I used to initialize my socket:

struct socket *sending_socket;
struct sock *sk;

   if (sock_create(PF_INET, SOCK_RAW, IPPROTO_RAW, &sending_socket) < 0)
   {
       printk("Error socket creation.\n");
       sock_release(sending_socket);
       return -1;
   }
   sk = kmalloc(sizeof(struct sock), GFP_KERNEL);
   memcpy(&(sending_socket->sk), sk, sizeof(struct sock));

I noticed that the sock_create function increments the reference count 
by two. When I copy the sending_socket->sk field in my sk variable, sk 
still gets this ref count =2. Thus when destroying the skbuff the socket 
is not freed (the release function decreases the sk ref count by one, 
see that there is one left, and exit without freeing the socket).
I also now think that I was doing the operations the wrong way around: I 
was trying to initialize all the skbuff fields, amongst whose was the 
socket. I discovered in some other codes that it is usually the socket 
which is first initialized and then the skbuff is attached to it. At 
least I'm now following that idea but I haven't had much time recently 
to go deeper on the implementation.

Thanks for your help,

        Emmanuel


Muli Ben-Yehuda wrote:

>On Mon, Feb 02, 2004 at 07:51:47PM +0200, Emmanuel Guiton wrote:
>
>  
>
>>However, my overall problem is not solved. As far as my investigations 
>>led me, my sk_buff structure is never released after having been sent on 
>>the wire. So I guess I need an explicit destructor function in my 
>>sk_buff as the following is present in the definition of struct sk_buff:
>>void         (*destructor)(struct sk_buff *);    /* Destruct function  */
>>    
>>
>
>Note that depending on what you're doing, you might not be able to use
>the destructor, because the upper layers use it without regards to
>whether it was set before. To the best of my understanding, the rules
>for the destructor say that it is free for the use of whatever layer
>owns the skbuff at the moment. There are three ways around it - the
>first and obvious is to avoid relying on the destructor. The second is
>that you can use skb_clone() to get your own copy of the headers and
>the destructor (but that doesn't really help you because how does the
>layer that ends up freeing the skb know to use your version of the
>headers?) and the third is to use this patch,
>http://www.mulix.org/patches/skb-destructor-chaining-A2-2.6.1, to 
>allow more than destructor per skb. 
>
>Hope this helps, 
>Muli 
>  
>

Re: Freeing skbuff (was: Re: Sending built-by-hand packet and kernel panic.)

[Duncan Sands]

> Thanks a lot for pointing out these problems. I had completely missed them.
> However, my overall problem is not solved. As far as my investigations
> led me, my sk_buff structure is never released after having been sent on
> the wire. So I guess I need an explicit destructor function in my
> sk_buff as the following is present in the definition of struct sk_buff:
> void         (*destructor)(struct sk_buff *);    /* Destruct function
>     */

Hi Emmanuel, maybe the call that sends to skb (NF_HOOK) is returning
a non-zero error code.  In that case it is your responsability [1] to free the
skb.

Duncan.

[1] This is true for many parts of the kernel but not all.  I don't know what
NF_HOOK is so this may not apply here.