| Message ID | 20180921182555.20362-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] flow: Fix uninitialized flow fields in IPv6 error case. | expand |
Thanks, looks good to me. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Fri, Sep 21, 2018 at 11:26 AM Ben Pfaff <blp@ovn.org> wrote: > When parse_ipv6_ext_hdrs__() returned false, half a 64-bit word had been > pushed into the miniflow and the second half was left uninitialized. This > commit fixes the problem. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518 > Signed-off-by > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518Signed-off-by>: > Ben Pfaff <blp@ovn.org> > --- > lib/flow.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/lib/flow.c b/lib/flow.c > index 128f64083ac7..bffee70ab55b 100644 > --- a/lib/flow.c > +++ b/lib/flow.c > @@ -868,11 +868,6 @@ miniflow_extract(struct dp_packet *packet, struct > miniflow *dst) > } > > tc_flow = get_16aligned_be32(&nh->ip6_flow); > - { > - ovs_be32 label = tc_flow & htonl(IPV6_LABEL_MASK); > - miniflow_push_be32(mf, ipv6_label, label); > - } > - > nw_tos = ntohl(tc_flow) >> 20; > nw_ttl = nh->ip6_hlim; > nw_proto = nh->ip6_nxt; > @@ -880,6 +875,12 @@ miniflow_extract(struct dp_packet *packet, struct > miniflow *dst) > if (!parse_ipv6_ext_hdrs__(&data, &size, &nw_proto, &nw_frag)) { > goto out; > } > + > + /* This needs to be after the parse_ipv6_ext_hdrs__() call > because it > + * leaves the nw_frag word uninitialized. */ > + ASSERT_SEQUENTIAL(ipv6_label, nw_frag); > + ovs_be32 label = tc_flow & htonl(IPV6_LABEL_MASK); > + miniflow_push_be32(mf, ipv6_label, label); > } else { > if (dl_type == htons(ETH_TYPE_ARP) || > dl_type == htons(ETH_TYPE_RARP)) { > -- > 2.16.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks, applied and backported. On Fri, Sep 21, 2018 at 04:01:59PM -0700, Yifeng Sun wrote: > Thanks, looks good to me. > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > On Fri, Sep 21, 2018 at 11:26 AM Ben Pfaff <blp@ovn.org> wrote: > > > When parse_ipv6_ext_hdrs__() returned false, half a 64-bit word had been > > pushed into the miniflow and the second half was left uninitialized. This > > commit fixes the problem. > > > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518 > > Signed-off-by > > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518Signed-off-by>: > > Ben Pfaff <blp@ovn.org> > > --- > > lib/flow.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/lib/flow.c b/lib/flow.c > > index 128f64083ac7..bffee70ab55b 100644 > > --- a/lib/flow.c > > +++ b/lib/flow.c > > @@ -868,11 +868,6 @@ miniflow_extract(struct dp_packet *packet, struct > > miniflow *dst) > > } > > > > tc_flow = get_16aligned_be32(&nh->ip6_flow); > > - { > > - ovs_be32 label = tc_flow & htonl(IPV6_LABEL_MASK); > > - miniflow_push_be32(mf, ipv6_label, label); > > - } > > - > > nw_tos = ntohl(tc_flow) >> 20; > > nw_ttl = nh->ip6_hlim; > > nw_proto = nh->ip6_nxt; > > @@ -880,6 +875,12 @@ miniflow_extract(struct dp_packet *packet, struct > > miniflow *dst) > > if (!parse_ipv6_ext_hdrs__(&data, &size, &nw_proto, &nw_frag)) { > > goto out; > > } > > + > > + /* This needs to be after the parse_ipv6_ext_hdrs__() call > > because it > > + * leaves the nw_frag word uninitialized. */ > > + ASSERT_SEQUENTIAL(ipv6_label, nw_frag); > > + ovs_be32 label = tc_flow & htonl(IPV6_LABEL_MASK); > > + miniflow_push_be32(mf, ipv6_label, label); > > } else { > > if (dl_type == htons(ETH_TYPE_ARP) || > > dl_type == htons(ETH_TYPE_RARP)) { > > -- > > 2.16.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/lib/flow.c b/lib/flow.c index 128f64083ac7..bffee70ab55b 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -868,11 +868,6 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) } tc_flow = get_16aligned_be32(&nh->ip6_flow); - { - ovs_be32 label = tc_flow & htonl(IPV6_LABEL_MASK); - miniflow_push_be32(mf, ipv6_label, label); - } - nw_tos = ntohl(tc_flow) >> 20; nw_ttl = nh->ip6_hlim; nw_proto = nh->ip6_nxt; @@ -880,6 +875,12 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) if (!parse_ipv6_ext_hdrs__(&data, &size, &nw_proto, &nw_frag)) { goto out; } + + /* This needs to be after the parse_ipv6_ext_hdrs__() call because it + * leaves the nw_frag word uninitialized. */ + ASSERT_SEQUENTIAL(ipv6_label, nw_frag); + ovs_be32 label = tc_flow & htonl(IPV6_LABEL_MASK); + miniflow_push_be32(mf, ipv6_label, label); } else { if (dl_type == htons(ETH_TYPE_ARP) || dl_type == htons(ETH_TYPE_RARP)) {
When parse_ipv6_ext_hdrs__() returned false, half a 64-bit word had been pushed into the miniflow and the second half was left uninitialized. This commit fixes the problem. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10518 Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/flow.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)