On Tue, Mar 22, 2016 at 06:02:35AM -0700, Justin Pettit wrote: > From: Ben Pfaff <b...@ovn.org> > > A crafted MPLS packet yields a zero 'count' in this excerpt from > miniflow_extract(): > > count = parse_mpls(&data, &size); > miniflow_push_words_32(mf, mpls_lse, mpls, count); > > In turn, miniflow_push_words_32() updated mf.map as follows: > > MF.map |= ((UINT64_MAX >> (64 - DIV_ROUND_UP(N_WORDS, 2))) << ofs64); > > which expanded to: > > mf.map |= (UINT64_MAX >> 64) << ofs64; > > Unforunately, C renders shifting a 64-bit constant by 64 bits undefined. > On common x86 platforms, 'n << 64' is equal to 'n', so this behaves as: > > mf.map |= UINT64_MAX << ofs64; > > In this particular case, ofs64 is 15, so this sets the most-significant 48 > bits of mf.map (a 63-bit bit-field) to 1. Only the least-significant 28 > bits of mf.map should ever be set to 1, so this sets 35 bits to 1 that > should never be. Because of the structure of the data structure that > mf.map is embedded within, this makes it possible later to overwrite 8*35 > == 280 bytes of data in the stack. However, there is no obvious way to > control the data used in the overwrite--it is memcpy'd from one place to > another but the source data does not come from the network. In the bug > reporter's testing, this overwrite caused a userspace crash if debug > logging was enabled, but not otherwise. > > This commit fixes the problem by avoiding the out-of-range shift. > > Vulnerability: CVE-2016-2074 > Reported-by: Kashyap Thimmaraju <kashyap.thimmar...@sec.t-labs.tu-berlin.de> > Reported-by: Bhargava Shastry <bshas...@sec.t-labs.tu-berlin.de> > Signed-off-by: Ben Pfaff <b...@ovn.org> > Acked-by: Jesse Gross <je...@kernel.org>
Jesse acked this one, privately. It's my patch so I can't ack it ;-) _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
