netdev - Re: Very Minor Patch to loopback.c

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Jun 2014 16:30:27 -0400
From:	Zachary <zacharyw09264@...il.com>
To:	Florian Fainelli <f.fainelli@...il.com>
Cc:	netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: Very Minor Patch to loopback.c

Should this be changed in dummy.c then to prevent the same bug?

On Thu, Jun 12, 2014 at 4:27 PM, Florian Fainelli <f.fainelli@...il.com> wrote:
> 2014-06-12 12:53 GMT-07:00 Zachary <zacharyw09264@...il.com>:
>> Very minor patch to loopback.c that I noticed while comparing the
>> dummy.c code to loopback.c code (dummy.c was based on loopback.c).  I
>> did not see a maintainer in MAINTAINERS in my cursory glance, so I
>> apologize if this is to the wrong place.  I am testing this patch
>> right now on my laptop and it appears to be working without issues.
>> It also compiled without warnings.
>
> Your patch introduces a potential use after free type of bug. As soon
> as the skb pointer has been passed to netif_rx(), the network stack is
> allowed to free this skb as soon as it has determined it is unused,
> this is the reason why this 'len' variable exists in the first place.
>
>>
>> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
>> index bb96409..f9f88d7 100644
>> --- a/drivers/net/loopback.c
>> +++ b/drivers/net/loopback.c
>> @@ -22,6 +22,7 @@
>>   *                                      interface.
>>   *             Alexey Kuznetsov:       Potential hang under some extreme
>>   *                                     cases removed.
>> + *             Zachary Winnerman:  Extremely minor patch in loopback_xmit
>>   *
>>   *             This program is free software; you can redistribute it and/or
>>   *             modify it under the terms of the GNU General Public License
>> @@ -72,7 +73,6 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
>>                                  struct net_device *dev)
>>  {
>>         struct pcpu_lstats *lb_stats;
>> -       int len;
>>
>>         skb_orphan(skb);
>>
>> @@ -86,10 +86,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
>>         /* it's OK to use per_cpu_ptr() because BHs are off */
>>         lb_stats = this_cpu_ptr(dev->lstats);
>>
>> -       len = skb->len;
>>         if (likely(netif_rx(skb) == NET_RX_SUCCESS)) {
>>                 u64_stats_update_begin(&lb_
>> stats->syncp);
>> -               lb_stats->bytes += len;
>> +               lb_stats->bytes += skb->len;
>>                 lb_stats->packets++;
>>                 u64_stats_update_end(&lb_stats->syncp);
>>         }
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html