Re: BUG: kernel NULL pointer dereference while copying sk_buff struct

From: Abdul Matin <abdulmatincuetcse17@gmail.com>
To: Alexander Kapshuk <alexander.kapshuk@gmail.com>
Cc: kernelnewbies <kernelnewbies@kernelnewbies.org>
Subject: Re: BUG: kernel NULL pointer dereference while copying sk_buff struct
Date: Tue, 2 May 2023 22:50:51 +0600	[thread overview]
Message-ID: <CAGFmy1Dfttv1hgj79s3vJxCk9-izxVQ9Xj=R+wbYMHB3jcEnKA@mail.gmail.com> (raw)
In-Reply-To: <CAJ1xhMW8GKQaU+6diUhH8XRab_KQyjGVYc356c99LQ_hV7kqfw@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 5370 bytes --]

Thanks for the tip!

On Tue, May 2, 2023 at 10:18 PM Alexander Kapshuk <
alexander.kapshuk@gmail.com> wrote:

> On Tue, May 2, 2023 at 6:35 PM Abdul Matin
> <abdulmatincuetcse17@gmail.com> wrote:
> >
> > I'm initializing the memory for skbPrev at module init function:
> >
> > static int __init nf_conntrack_my_mod_init(void)
> > {
> >     saddr_m = (union nf_inet_addr* ) kmalloc(sizeof(union nf_inet_addr),
> GFP_KERNEL);
> >     skbPrev = (struct sk_buff*) kmalloc(sizeof(struct sk_buff),
> GFP_KERNEL);
> >     if (saddr_m == NULL) {
> >       printk(KERN_INFO "Can not allocate space for saddr\n");
> >       return -ENOMEM;
> >     }
> >     if(skbPrev == NULL) {
> >       printk(KERN_INFO "Can not allocate space for skbPrev\n");
> >       return -ENOMEM;
> >     }
> >
> > On Tue, May 2, 2023 at 8:23 PM Alexander Kapshuk <
> alexander.kapshuk@gmail.com> wrote:
> >>
> >> On Tue, May 2, 2023 at 3:26 PM Abdul Matin
> >> <abdulmatincuetcse17@gmail.com> wrote:
> >> >
> >> > Hi.
> >> > I'm writing a netfilter module where I need to copy a sk_buff in a
> global variable that I use in another subsequent call. But I crashed the
> whole kernel. I've tried to add a code snippet to share with you how I'm
> doing it.
> >> >
> >> > here case1 is always true before case2 (i.e. 1st call of help ->
> case1 is true, 2nd call of help -> case2 true).
> >> > So, in the 2nd call, case2 is true where we're using exp, ctinfoPrev,
> saddr_m which have been initialized before in case1.
> >> >
> >> > union nf_inet_addr *saddr_m;
> >> > struct sk_buff* skbPrev;
> >>
> >> This declares a pointer to struct sk_buff which is uninitialised and
> >> most probably set to NULL by the compiler.
> >> Where do you allocate memory for skbPrev?
> >>
> >> > enum ip_conntrack_info ctinfoPrev;
> >> > struct nf_conntrack_expect *exp;
> >> >
> >> > static int help(struct sk_buff *skb,
> >> >     unsigned int protoff,
> >> >     struct nf_conn *ct,
> >> >     enum ip_conntrack_info ctinfo)
> >> > {
> >> >      switch (msgType) {
> >> >     case case1:
> >> >
> >> >     ctinfoPrev = ctinfo;
> >> >     memcpy((void *)skbPrev, (const void *)skb, sizeof(skb));
> >>
> >> The NULL pointer dereference probably happens here, as memcpy attempts
> >>  to copy data from skb to skbPrev, which is likely to be NULL.
> >>
> >> >     skbPrev->next = (struct sk_buff*) kmalloc(sizeof(struct sk_buff),
> GFP_KERNEL);
> >> >     skbPrev->prev = (struct sk_buff*) kmalloc(sizeof(struct sk_buff),
> GFP_KERNEL);
> >> >     skbPrev->sk = (struct sock*) kmalloc(sizeof(struct sock),
> GFP_KERNEL);
> >> >     memcpy((void *)(skbPrev->next), (const void *)skb->next,
> sizeof(skb->next));
> >> >     memcpy((void *)(skbPrev->prev), (const void *)skb->prev,
> sizeof(skb->prev));
> >> >    memcpy((void *)(skbPrev->sk), (const void *)skb->sk,
> sizeof(skb->sk));
> >> >
> >> >     unsigned int type = (dptr[0] << 8) | dptr[1]; // little endian
> >> >     unsigned int length = (dptr[2] << 8) | dptr[3];
> >> >     printk(KERN_INFO "type: %hu length: %hu", type, length);
> >> >
> >> >     unsigned int ip;
> >> >     memcpy(&ip, dptr, 4);
> >> >     ip = ntohl(ip) ^ MAGIC_COOKIE_VALUE_HOST;
> >> >     exp = nf_ct_expect_alloc(ct);
> >> >     if (exp == NULL) {
> >> >         printk( KERN_INFO "cannot alloc expectation");
> >> >         return NF_DROP;
> >> >     }
> >> >     tuple = &ct->tuplehash[IP_CT_DIR_REPLY].tuple;
> >> >     nf_ct_expect_init(exp, NF_CT_EXPECT_CLASS_DEFAULT,
> >> >     nf_ct_l3num(ct),
> >> >     saddr_m, &tuple->dst.u3,
> >> >     IPPROTO_UDP, NULL, &tuple->dst.u.udp.port);
> >> >
> >> >     pr_debug("expect: ");
> >> >     nf_ct_dump_tuple(&exp->tuple);
> >> >
> >> >
> >> >     break;
> >> >    case case2:
> >> >     printk(KERN_INFO "createpermission response\n");
> >> >     nf_nat_tftp = rcu_dereference(nf_nat_tftp_hook);
> >> >     if (nf_nat_tftp && ct->status & IPS_NAT_MASK)
> >> >     ret= nf_nat_tftp(skbPrev, ctinfoPrev, exp);
> >> >     else if (nf_ct_expect_related(exp, 0) != 0) {
> >> >     printk( KERN_INFO "cannot add expectation");
> >> >     nf_ct_helper_log(skb, ct, "cannot add expectation");
> >> >     ret = NF_DROP;
> >> >     }
> >> >    nf_ct_expect_put(exp);
> >> >    break;
> >> >    }
> >> >    return ret;
> >> >   }
> >> > I got this log before crash: 1,589,5743337757,-;BUG: kernel NULL
> pointer dereference, address: 0000000000000000
> >> > 1,590,5743337860,-;#PF: supervisor read access in kernel mode
> >> > 1,591,5743337880,-;#PF: error_code(0x0000) - not-present page
> >> > 6,592,5743337900,-;PGD 0 P4D 0
> >> > 4,593,5743337974,-;Oops: 0000 [#1] SMP PTI
> >> >
> >> > Is there anything wrong I am doing in copying and initializing?
> >> >
> >> > _______________________________________________
> >> > Kernelnewbies mailing list
> >> > Kernelnewbies@kernelnewbies.org
> >> > https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>
> I see.
> Without seeing the code in its entirety it's hard to tell which
> pointer is being null-dereferenced.
> Examine the OOPS output. The answer may very well be there.
> Otherwise, you want to add some pr_info() or printk() calls around the
> pointers being dereferenced in your code and see if any of those are
> set to NULL.
>

[-- Attachment #1.2: Type: text/html, Size: 7453 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies