netdev - Re: net_tx_action race condition?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <7f70cdb4-4205-169a-0204-fd5cd72b44f1@gmail.com>
Date:   Wed, 28 Mar 2018 09:32:18 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Saurabh Kr <krsaurabh@...iper.net>,
        Angelo Rizzi <angelo.rizzi@...utomazione.it>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sarvendra Vikram Singh <svikrams@...iper.net>,
        Kunal Sharma <skunal@...iper.net>
Subject: Re: net_tx_action race condition?



On 03/28/2018 12:30 AM, Saurabh Kr wrote:
> Hi Eric/Angelo,
>  
> We are seeing the assertion error  in linux kernel 2.4.29  “*kernel: KERNEL: assertion (atomic_read(&skb->users) == 0) failed at dev.c(1397)**”.* Based on patch provided (_https://patchwork.kernel.org/patch/5368051/_ ) we merged the changes in linux kernel 2.4.29 but we are still facing the assertion error at dev.c (1397). Please let me know your thoughts.
>  
> *Before Merge**(linux 2.4.29)*
> ---------------------------------
>  
> static void net_tx_action(struct softirq_action *h)
> {
>         int cpu = smp_processor_id();
>  
>         if (softnet_data[cpu].completion_queue) {
>                 struct sk_buff *clist;
>  
>                 local_irq_disable();
>                 clist = softnet_data[cpu].completion_queue; // Existing code
>                 softnet_data[cpu].completion_queue = NULL;
>                 local_irq_enable();
>  
>                 while (clist != NULL) {
>                         struct sk_buff *skb = clist;
>                         clist = clist->next;
>  
>                         BUG_TRAP(atomic_read(&skb->users) == 0);
>                         __kfree_skb(skb);
>                 }
>         }
>  
>          ---------
>  
> *After Merge the changes based on available patch**(linux 2.4.29)**:*
> ------------------------------------------------------------------------------
>  
> static void net_tx_action(struct softirq_action *h)
> {
>         int cpu = smp_processor_id();
>  
>         if (softnet_data[cpu].completion_queue) {
>                 struct sk_buff *clist;
>  
>                 local_irq_disable();
>                 clist = *(volatile typeof(softnet_data[cpu].completion_queue) *)&( softnet_data[cpu].completion_queue);  // Modified line based on available patch
>                 softnet_data[cpu].completion_queue = NULL;
>                 local_irq_enable();
>  
>                 while (clist != NULL) {
>                         struct sk_buff *skb = clist;
>                         clist = clist->next;
>  
>                         BUG_TRAP(atomic_read(&skb->users) == 0);
>                         __kfree_skb(skb);
>                 }
>         }
>   ………….
>  
> Thanks & regards,
> Saurabh
>  

Thats simply prove (again) that this 'fix' was not the proper one.

I have no idea what is wrong, and there is no way I am going to look at 2.4.29 kernel...