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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 24 Mar 2014 11:00:17 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Alon Nafta' <alon@...vatecore.com>,
	netdev <netdev@...r.kernel.org>
CC:	Grant Grundler <grantgrundler@...il.com>
Subject: RE: [PATCH 1/4 V2] Ethernet drivers in 3.14-rc3 kernel: fix 3
 buffer overflows triggered by hardware devices

From: Alon Nafta
> Hi,
> 
> Has anyone looked at this patch?
> 
> Thanks,
> Alon
> 
> On Mon, Feb 24, 2014 at 10:11 AM, Alon Nafta <alon@...vatecore.com> wrote:
> > From: Alon Nafta <alon@...vatecore.com>
> >
> > Linux Kernel 3.14 contains multiple overflow conditions that are triggered
> > as hardware inputs are not properly validated when parsing Ethernet packets.
> > This may allow a local attacker to cause an overflow, resulting in a denial
> > of service or potentially allowing the execution of arbitrary code.

These are lengths written by hardware, so will only be wrong if the
hardware is broken.
If the hardware is broken (or replaced by something malicious)
then it can do anything it likes.
Invalid values in ring entries are the least of your worries.

It also isn't clear that generating an overlong packet is
any better.

> > The programmatic error resides in the use of an integer type to describe
> > packet lengths, without proper validation to disallow negative values. In
> > all three (3) bugs this patch fixes, a value of 0x30000 of the hardware
> > signal, named status, will result in a value of 0xffffffff for pkt_len, and
> > an allocation of a socket buffer with size of 0x1. This results in an
> > overflow when data is copied into that buffer.
> >
> > Signed-off-by: Alon Nafta <alon@...vatecore.com>
> > Reviewed-by: Grant Grundler <grantgrundler@...il.com>
> > ---
> > diff -uprN -X linux-3.14-rc3/Documentation/dontdiff
> > linux-3.14-rc3-orig/drivers/net/ethernet/dec/tulip/de4x5.c
> > linux-3.14-rc3/drivers/net/ethernet/dec/tulip/de4x5.c
> > --- linux-3.14-rc3-orig/drivers/net/ethernet/dec/tulip/de4x5.c 2014-02-20
> > 17:59:14.704084300 -0800
> > +++ linux-3.14-rc3/drivers/net/ethernet/dec/tulip/de4x5.c 2014-02-20
> > 18:23:08.987749400 -0800
> > @@ -1635,8 +1635,8 @@ de4x5_rx(struct net_device *dev)
> >   if (status & RD_OF)           lp->pktStats.rx_overflow++;
> >       } else {                          /* A valid frame received */
> >   struct sk_buff *skb;
> > - short pkt_len = (short)(le32_to_cpu(lp->rx_ring[entry].status)
> > -                             >> 16) - 4;
> > + short pkt_len = (short)((le32_to_cpu(lp->rx_ring[entry].status)
> > +                             >> 16) - 4) & 0x7fff;

There isn't much point masking with 0x7fff and then casting to (short).
Actually changing the variable to 'unsigned int' might generate
better code.

	David



--
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

Powered by blists - more mailing lists