[ovs-dev,ovn] pinctrl: Fix buffer overread in pinctrl_compose_ipv6().
diff mbox series

Message ID 20200224233914.697716-1-blp@ovn.org
State New
Headers show
Series
  • [ovs-dev,ovn] pinctrl: Fix buffer overread in pinctrl_compose_ipv6().
Related show

Commit Message

Ben Pfaff Feb. 24, 2020, 11:39 p.m. UTC
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(+)

Comments

Dumitru Ceara Feb. 25, 2020, 9:13 a.m. UTC | #1
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;
>
Numan Siddique Feb. 25, 2020, 9:32 a.m. UTC | #2
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
>
Numan Siddique Feb. 27, 2020, 7:42 p.m. UTC | #3
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
> >

Patch
diff mbox series

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;