Search code examples
memory-managementlinux-kerneliptables

How can I safely append data to a sk_buff for IPTables target


I am working on a Linux kernel module that needs to modify network packets and append an extra header. I already implemented the modification part, recomputed the check-sums and it worked nice. But I don't know how to safely append an extra header. If my input packet is something like:

ip-header / tcp-header / data

I would like to have an output packet like:

ip-header / tcp-header / my-header / data

For what I read, I think I need something like the following code. I wrote my specific questions on the code as comments. My general concern is if the code I am writing here is memory-safe or what should I do to have a memory-safe way to append the new header. Also, if I am doing something wrong or there is a better way to do it I will also appreciate the comment. I have tried to find examples but no luck so far. Here is the code:

static unsigned int my_iptables_target(struct sk_buff *skb, const struct xt_action_param *par) {
    const struct xt_mytarget_info *info = par->targinfo;
    /* Some code ... */

    if (!skb_make_writable(skb, skb->len)) {
        //Drop the packet
        return NF_DROP;
    }

    struct newheader* myheader;
    // Check if there is enough space and do something about it
    if (skb_headroom(skb) < sizeof(struct newheader)) {
        // So there is no enugh space.
        /* I don't know well what to put here. I read that a function called pskb_expand_head might
         * do the job. I do not understand very well how it works, or why it might fail (return value
         * different from zero). Does this code work:
         */
        if (pskb_expand_head(skb, sizeof(struct newheader) - skb_headroom(skb), 0, GPF_ATOMIC) != 0) {
            // What does it mean if the code reaches this point?
            return NF_DROP;
        }
    }
    // At this point, there should be enough space
    skb_push(skb, sizeof(struct newheader));

    /* I also think that skb_push() creates space at the beggining, to open space between the header and
     * the body I guess I must move the network/transport headers up. Perhaps something like this:
     */
    memcpy(skb->data, skb->data + sizeof(struct newheader), size_of_all_headers - sizeof(struct newheader));

    // Then set myheader address and fill data.
    myheader = skb->data + size_of_all_headers;

    //Then just set the new header, and recompute checksums.

    return XT_CONTINUE;
}

I assumed that the variable size_of_all_headers contains the size in bytes of the network and transport headers. I also think that memcpy copies bytes in increasing order, so that call shouldn't be a problem. So does the above code works? It is all memory-safe? Are there better ways to do it? Are there examples (or can you provide one) that does something like this?


Solution

  • I used a code similar to the one in the question and so far it has worked very well for all the test I have done. To answer some of the specific questions, I used something like:

    if (skb_headroom(skb) < sizeof(struct newheader)) {
        printk("I got here!\n");
        if (pskb_expand_head(skb, sizeof(struct newheader) - skb_headroom(skb), 0, GPF_ATOMIC) != 0) {
            printk("And also here\n");
            return NF_DROP;
        }
    }
    

    But none of the print statements ever executed. I suppose that happens because the OS reserves enough space in memory such that there can be no problems given the limits of the IP header. But I think it is better to leave that if statement to grow the packet if necessary.

    The other difference of the code that I tested and worked is that instead of moving all the other headers up to create a space for my header, I chose to move the body of the packet down.