| Message ID | 20200224233914.697716-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] pinctrl: Fix buffer overread in pinctrl_compose_ipv6(). | expand |
On 2/25/20 12:39 AM, Ben Pfaff wrote: > The call to packet_set_ipv6() calls into packet_rh_present(), which in > turn iterates through the L3 content. Without this commit, the l4_ofs > in the packet has its default value of UINT16_MAX, which means that > packet_rh_present() reads well beyond the real maximum length of the > IPv6 header. Oops, sorry about that. Thanks for fixing it! > > Reported by Address Sanitizer (actually the MLD test fails 100% of the > time with Address Sanitizer without this fix, so I guess I'm the only > one who uses it routinely). I'll try to run it regularly from now on. > > CC: Dumitru Ceara <dceara@redhat.com> > Fixes: 677a3ba4d66b ("ovn: Add MLD support.") > Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Dumitru Ceara <dceara@redhat.com> > --- > controller/pinctrl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index d06915a65173..dc8d3fd28ea0 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -3238,6 +3238,7 @@ pinctrl_compose_ipv6(struct dp_packet *packet, struct eth_addr eth_src, > eh->eth_src = eth_src; > eh->eth_type = htons(ETH_TYPE_IPV6); > dp_packet_set_l3(packet, nh); > + dp_packet_set_l4(packet, nh + 1); > > nh->ip6_vfc = 0x60; > nh->ip6_nxt = ip_proto; >
On Tue, Feb 25, 2020 at 2:43 PM Dumitru Ceara <dceara@redhat.com> wrote: > > On 2/25/20 12:39 AM, Ben Pfaff wrote: > > The call to packet_set_ipv6() calls into packet_rh_present(), which in > > turn iterates through the L3 content. Without this commit, the l4_ofs > > in the packet has its default value of UINT16_MAX, which means that > > packet_rh_present() reads well beyond the real maximum length of the > > IPv6 header. > > Oops, sorry about that. Thanks for fixing it! > > > > > Reported by Address Sanitizer (actually the MLD test fails 100% of the > > time with Address Sanitizer without this fix, so I guess I'm the only > > one who uses it routinely). > > I'll try to run it regularly from now on. Me too. > > > > > CC: Dumitru Ceara <dceara@redhat.com> > > Fixes: 677a3ba4d66b ("ovn: Add MLD support.") > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > Acked-by: Dumitru Ceara <dceara@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Thanks Numan > > > --- > > controller/pinctrl.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index d06915a65173..dc8d3fd28ea0 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -3238,6 +3238,7 @@ pinctrl_compose_ipv6(struct dp_packet *packet, struct eth_addr eth_src, > > eh->eth_src = eth_src; > > eh->eth_type = htons(ETH_TYPE_IPV6); > > dp_packet_set_l3(packet, nh); > > + dp_packet_set_l4(packet, nh + 1); > > > > nh->ip6_vfc = 0x60; > > nh->ip6_nxt = ip_proto; > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Feb 25, 2020 at 3:02 PM Numan Siddique <numans@ovn.org> wrote: > > On Tue, Feb 25, 2020 at 2:43 PM Dumitru Ceara <dceara@redhat.com> wrote: > > > > On 2/25/20 12:39 AM, Ben Pfaff wrote: > > > The call to packet_set_ipv6() calls into packet_rh_present(), which in > > > turn iterates through the L3 content. Without this commit, the l4_ofs > > > in the packet has its default value of UINT16_MAX, which means that > > > packet_rh_present() reads well beyond the real maximum length of the > > > IPv6 header. > > > > Oops, sorry about that. Thanks for fixing it! > > > > > > > > Reported by Address Sanitizer (actually the MLD test fails 100% of the > > > time with Address Sanitizer without this fix, so I guess I'm the only > > > one who uses it routinely). > > > > I'll try to run it regularly from now on. > > Me too. > > > > > > > > CC: Dumitru Ceara <dceara@redhat.com> > > > Fixes: 677a3ba4d66b ("ovn: Add MLD support.") > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > > > Acked-by: Dumitru Ceara <dceara@redhat.com> > > Acked-by: Numan Siddique <numans@ovn.org> I applied this patch to master and branch-20.03 Thanks Numan > > Thanks > Numan > > > > > --- > > > controller/pinctrl.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > > index d06915a65173..dc8d3fd28ea0 100644 > > > --- a/controller/pinctrl.c > > > +++ b/controller/pinctrl.c > > > @@ -3238,6 +3238,7 @@ pinctrl_compose_ipv6(struct dp_packet *packet, struct eth_addr eth_src, > > > eh->eth_src = eth_src; > > > eh->eth_type = htons(ETH_TYPE_IPV6); > > > dp_packet_set_l3(packet, nh); > > > + dp_packet_set_l4(packet, nh + 1); > > > > > > nh->ip6_vfc = 0x60; > > > nh->ip6_nxt = ip_proto; > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index d06915a65173..dc8d3fd28ea0 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -3238,6 +3238,7 @@ pinctrl_compose_ipv6(struct dp_packet *packet, struct eth_addr eth_src, eh->eth_src = eth_src; eh->eth_type = htons(ETH_TYPE_IPV6); dp_packet_set_l3(packet, nh); + dp_packet_set_l4(packet, nh + 1); nh->ip6_vfc = 0x60; nh->ip6_nxt = ip_proto;
The call to packet_set_ipv6() calls into packet_rh_present(), which in turn iterates through the L3 content. Without this commit, the l4_ofs in the packet has its default value of UINT16_MAX, which means that packet_rh_present() reads well beyond the real maximum length of the IPv6 header. Reported by Address Sanitizer (actually the MLD test fails 100% of the time with Address Sanitizer without this fix, so I guess I'm the only one who uses it routinely). CC: Dumitru Ceara <dceara@redhat.com> Fixes: 677a3ba4d66b ("ovn: Add MLD support.") Signed-off-by: Ben Pfaff <blp@ovn.org> --- controller/pinctrl.c | 1 + 1 file changed, 1 insertion(+)