On Wed, Dec 26, 2018 at 04:52:22PM -0800, Yifeng Sun wrote: > In this piece of code, 'struct ofpbuf b' should always point to > metadata so that metadata can be filled with values through ofpbuf > operations, like ofpbuf_put_hex and ofpbuf_push_zeros. However, > ofpbuf_push_zeros may change the data pointer of 'struct ofpbuf b' > and therefore, metadata will not contain the expected values. This > patch fixes it. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10863 > Signed-off-by: Yifeng Sun <[email protected]> > --- > lib/odp-util.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index cb6a5f2047fd..af855873690c 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -2114,12 +2114,12 @@ parse_odp_push_nsh_action(const char *s, struct > ofpbuf *actions) > if (ovs_scan_len(s, &n, "md2=0x%511[0-9a-fA-F]", buf) > && n/2 <= sizeof metadata) { > ofpbuf_use_stub(&b, metadata, sizeof metadata); > - ofpbuf_put_hex(&b, buf, &mdlen); > /* Pad metadata to 4 bytes. */ > padding = PAD_SIZE(mdlen, 4); > if (padding > 0) { > - ofpbuf_push_zeros(&b, padding); > + ofpbuf_put_zeros(&b, padding); > } > + ofpbuf_put_hex(&b, buf, &mdlen); > md_size = mdlen + padding; > ofpbuf_uninit(&b); > continue;
Yifeng, this fix looks wrong because it uses 'mdlen' in PAD_SIZE before initializing it. This code is weird. It adds padding to a 4-byte boundary even though I can't find any other code that checks for that or relies on it. Furthermore, it puts the padding **BEFORE** the metadata, which just seems super wrong. When I look at datapath code for md2 I get even more confused. nsh_key_put_from_nlattr() seems to assume that an md2 attribute has well-formatted data in it, then nsh_hdr_from_nlattr() copies it without checking into nh->md2, and then if it's not perfectly formatted then nsh->md2.length is going to be invalid. If I'm reading it right, it also assumes there's exactly one TLV. Jan, I think this is your code, can you help me understand this code? Thanks, Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
