netdev - Re: Revert 462fb2af9788a82a534f8184abfde31574e1cfa0 (bridge : Sanitize skb before it enters the IP stack)

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]

Message-ID: <1400253599.7973.181.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Fri, 16 May 2014 08:19:59 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Newall <davidn@...idnewall.com>
Cc:	Lukas Tribus <luky-37@...mail.com>,
	Netdev <netdev@...r.kernel.org>, "fw@...len.de" <fw@...len.de>
Subject: Re: Revert 462fb2af9788a82a534f8184abfde31574e1cfa0 (bridge :
 Sanitize skb before it enters the IP stack)

On Sat, 2014-05-17 at 00:03 +0930, David Newall wrote:
> We should revert commit 462fb2af9788a82a534f8184abfde31574e1cfa0 (bridge 
> : Sanitize skb before it enters the IP stack) because it corrupts IP 
> packets with RR or TS options set, only partially updating the IP header 
> and leaving an incorrect checksum.
> 
> The argument for introducing the change is at lkml.org/lkml/2010/8/30/391:
> 
> The bridge layer can overwrite the IPCB using the
> BR_INPUT_SKB_CB macro. In br_nf_dev_queue_xmit,
> if we recieve a packet greater in size than the bridge
> device MTU, we call ip_fragment which in turn will lead to
> icmp_send calling ip_options_echo if the DF flag is set.
> ip_options_echo will then incorrectly try to parse the IPCB as
> IP options resulting in a buffer overflow.
> This change refills the CB area back with IP
> options before ip_fragment calls icmp_send. If we fail parsing,
> we zero out the IPCB area to guarantee that the stack does
> not get corrupted.
> 
> A bridge should not fragment packets; an *ethernet* bridge should not 
> need to.  Fragmenting packets is the job of higher level protocol.
> 
> --- br_netfilter.c	2014-01-20 13:10:07.000000000 +1030
> +++ br_netfilter.c.prop	2014-05-16 23:07:57.975386905 +0930


Is this meant to be an official patch ?

Please 

1) CC patch author (Bandan Das <bandan.das@...atus.com>) so he has a way
to comment if he no longer follows netdev ?

2) Read Documentation/SubmittingPatches 

>   	return NF_STOLEN;
> +
> +inhdr_error:
> +//      IP_INC_STATS_BH(IpInHdrErrors);

Don't leave this // 

Thanks


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