[ovs-dev] [PATCH v3 3/6] ovn: Add a new OVN field icmp4.frag_mtu

Numan Siddique nusiddiq at redhat.com
Fri Apr 19 18:55:14 UTC 2019


On Fri, Apr 19, 2019 at 3:27 AM Ben Pfaff <blp at ovn.org> wrote:

> On Thu, Apr 18, 2019 at 02:55:58PM -0700, Ben Pfaff wrote:
> > This calculation in pinctrl_handle_icmp() looks suspect to me.  Does it
> > function properly if the packet is larger than about 576 bytes and the
> > subtraction wraps around?
> >
> > +            /* Calculate available room to include the original IP +
> data. */
> > +            nh = dp_packet_l3(&packet);
> > +            uint16_t room = 576 - (sizeof *eh + ntohs(nh->ip_tot_len));
> > +            if (in_ip_len > room) {
> > +                in_ip_len = room;
> > +            }
>
> Excuse me: the above is actually a comment for patch 4.
>

Hi Ben,

Based on the code here -
https://github.com/openvswitch/ovs/blob/master/ovn/controller/pinctrl.c#L557

i.e
*****
 nh->ip_tot_len = htons(sizeof(struct ip_header) +
                                sizeof(struct icmp_header));
*****

The value of  sizeof *eh + nh->ip_tot_len can never be greater than 576.
So you think it's still not safe and it's better to handle the subtraction
wrap ?

Thanks
Numan


More information about the dev mailing list