Ok. I'll find some time tomorrow to push a patch fixing both v4 and v6.

-----Original Message-----
From: Klement Sekera <ksek...@cisco.com> 
Sent: Tuesday, September 25, 2018 6:02 PM
To: Kingwel Xie <kingwel....@ericsson.com>; vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] A bug in IP reassembly?

Hi Kingwel,

thanks for finding this bug. Your patch looks fine - would you mind making a 
similar fix in ip4_reassembly.c? The logic suffers from the same flaw there.

Thanks,
Klement

Quoting Kingwel Xie (2018-09-25 11:06:49)
>    Hi,
> 
>     
> 
>    I worked on testing IP reassembly recently, the hit a crash when testing
>    IP reassembly with IPSec. It took me some time to figure out why.
> 
>     
> 
>    The crash only happens when there are >1 feature node enabled under
>    ip-unicast and ip reassembly is working, like below.
> 
>     
> 
>    ip4-unicast:
> 
>      ip4-reassembly-feature
> 
>      ipsec-input-ip4
> 
>     
> 
>    It looks like there is a bug in the reassembly code as below:
>    vnet_feature_next will do to buffer b0 to update the next0 and the
>    current_config_index of b0, but b0 is pointing to some fragment buffer
>    which in most cases is not the first buffer in chain indicated by bi0.
>    Actually bi0 pointing to the first buffer is returned by ip6_reass_update
>    when reassembly is finalized. As I can see this is a mismatch that bi0 and
>    b0 are not the same buffer. In the end the quick fix is like what I added
>    : b0 = vlib_get_buffer (vm, bi0); to make it right.
> 
>     
> 
>                      if (~0 != bi0)
> 
>                        {
> 
>                        skip_reass:
> 
>                          to_next[0] = bi0;
> 
>                          to_next += 1;
> 
>                          n_left_to_next -= 1;
> 
>                          if (is_feature && IP6_ERROR_NONE == error0)
> 
>                                    {
> 
>                                      b0 = vlib_get_buffer (vm, bi0);  à added
>    by Kingwel
> 
>                                      vnet_feature_next (&next0, b0);
> 
>                                    }
> 
>                          vlib_validate_buffer_enqueue_x1 (vm, node,
>    next_index, to_next,
> 
>                                                                               
>     
>           n_left_to_next, bi0, next0);
> 
>                        }
> 
>     
> 
>    Probably this is not the perfect fix, but it works at least. Wonder if
>    committers have better thinking about it? I can of course push a patch if
>    you think it is ok.
> 
>     
> 
>    Regards,
> 
>    Kingwel
> 
>     
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#10648): https://lists.fd.io/g/vpp-dev/message/10648
Mute This Topic: https://lists.fd.io/mt/26218556/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to