null pointer deref issue while executing skb_release_data

[vdx]

Dear Friends,

I am building a packet forwarder kern-mod. for that I did add prerouting hook function in netfilter.
In the same function, on successful match I am stilling that same. For that I use below function,

/* Transmitter function */
static int my_xmit(struct sk_buff *skb)
{
        struct rtable *rt;                      // route to the other host
        struct iphdr  *iph = skb->nh.iph;
        u8    tos = iph->tos;
        int    mtu;
        struct flowi flp;

        memset(&flp, 0, sizeof(flp));
        flp.fl4_dst = iph->daddr;
        flp.fl4_tos = RT_TOS(tos);

        if(ip_route_output_flow(&rt, &flp, NULL, 0)) {

                printk(KERN_ERR  "my_xmit[%d]: ip_route_output error, dest: %u.%u.%u.%u\n",__LINE__,NIPQUAD(iph->daddr));
                goto tx_error_icmp;
        }

        /* MTU checking ??? */
        mtu = dst_mtu(&rt->u.dst);
        if ((skb->len > mtu) && (iph->frag_off&__constant_htons(IP_DF))) {

                icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
                ip_rt_put(rt);

                printk(KERN_ERR "my_xmit[%d]: frag needed\n",__LINE__);
                goto tx_error;
        }

        /* drop old route */
        dst_release(skb->dst);
        skb->dst = &rt->u.dst;

        dst_output(skb);
        return NF_STOLEN;

  tx_error_icmp:
        dst_link_failure(skb);
  tx_error:
        kfree_skb(skb);
        return NF_STOLEN;
}

the issue with above is, at some point of time my system gets crash. Below is crash dump utility analysis

crash> bt
PID: 0      TASK: ffff81022fd0f040  CPU: 16  COMMAND: "swapper"
#0 [ffff81012fe0b9b0] crash_kexec at ffffffff800b093a
#1 [ffff81012fe0ba70] __die at ffffffff80065137
#2 [ffff81012fe0bab0] do_page_fault at ffffffff80067484
#3 [ffff81012fe0bba0] error_exit at ffffffff8005dde9
    [exception RIP: my_prerouting_hook+1044]
    RIP: ffffffff8845a475  RSP: ffff81012fe0bc50  RFLAGS: 00010206
    RAX: ffffc200111b5000  RBX: ffff81012fe0bd50  RCX: 0000000000000000
    RDX: ffff81010a591044  RSI: 0000000000000014  RDI: ffff81011be607c0
    RBP: 0000000007be4e60  R8: ffffffff80254a2c  R9: ffffffff8845d3c0
    R10: ffff81010f52b080  R11: 00000000000000c8  R12: ffff81012b5a4000
    R13: 00000000000ffff0  R14: ffff81012fe0bda0  R15: ffffffff805c7ac0
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
#4 [ffff81012fe0bce8] nf_iterate at ffffffff80033faf
#5 [ffff81012fe0bd10] ip_rcv_finish at ffffffff80254a2c
#6 [ffff81012fe0bd88] ip_rcv at ffffffff8003550b
#7 [ffff81012fe0bdc8] netif_receive_skb at ffffffff80020aed
#8 [ffff81012fe0be18] tg3_poll at ffffffff881b0be7 [tg3]
#9 [ffff81012fe0bef8] net_rx_action at ffffffff8000c9bf
#10 [ffff81012fe0bf38] __do_softirq at ffffffff80012531
#11 [ffff81012fe0bf68] call_softirq at ffffffff8005e2fc
#12 [ffff81012fe0bf80] do_softirq at ffffffff8006d646
#13 [ffff81012fe0bf90] do_IRQ at ffffffff8006d4d6

crash> dis -lr ffffffff8845a475
0xffffffff8845a061 <my_prerouting_hook>: push  %r14
0xffffffff8845a0bb <my_prerouting_hook+90>:      mov    %rbx,%rdx
0xffffffff8845a0be <my_prerouting_hook+93>:      callq  0xffffffff80233be4 <skb_copy_bits+317>
0xffffffff8845a218 <my_prerouting_hook+439>:    callq  0xffffffff80048106 <mpage_end_io_read+71>
0xffffffff8845a23b <my_prerouting_hook+474>:    callq  0xffffffff802509ae <nf_register_hooks+11>
0xffffffff8845a29e <my_prerouting_hook+573>:    callq  0xffffffff80011fec <do_munmap+634>
0xffffffff8845a305 <my_prerouting_hook+676>:    callq  0xffffffff8025713a <ip_copy_metadata+167>
0xffffffff8845a31e <my_prerouting_hook+701>:    callq  0xffffffff8006201c <__memset>
0xffffffff8845a33d <my_prerouting_hook+732>:    callq  0xffffffff80253106 <ip_route_output_flow+316>
0xffffffff8845a370 <my_prerouting_hook+783>:    callq  0xffffffff8026ab1a <icmp_send+326>
0xffffffff8845a3a2 <my_prerouting_hook+833>:    callq  0xffffffff80094387 <printk+3>
0xffffffff8845a3a7 <my_prerouting_hook+838>:    callq  0xffffffff8006c4a1 <dump_stack>
0xffffffff8845a3f4 <my_prerouting_hook+915>:    callq  0xffffffff80094387 <printk+3>
0xffffffff8845a3f9 <my_prerouting_hook+920>:    callq  0xffffffff8006c4a1 <dump_stack>
0xffffffff8845a43a <my_prerouting_hook+985>:    callq  0xffffffff80094387 <printk+3>

0xffffffff8845a465 <my_prerouting_hook+1028>:    callq  0xffffffff802344b1 <skb_release_data+150>
0xffffffff8845a46a <my_prerouting_hook+1033>:    mov    $0x2,%eax
0xffffffff8845a46f <my_prerouting_hook+1038>:    jmp    0xffffffff8845a4a8
0xffffffff8845a471 <my_prerouting_hook+1040>:    mov    0x0(%rbp),%rbp
0xffffffff8845a475 <my_prerouting_hook+1044>:    mov    0x0(%rbp),%rax

I getting NULL pointer deref issue in above case. how do I resolve ?

 

[GrumpyOldMan]

First thing I would do is verify that any pointer object you dereference is valid. This includes arguments to functions. For example, in the function shown above, you're dereferencing skb without doing a sanity check. And then, you're dereferencing the value you obtained from a field in skb again without checking, all within the declaration block. Paranoia is your friend here.