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 (#10642): https://lists.fd.io/g/vpp-dev/message/10642
Mute This Topic: https://lists.fd.io/mt/26218556/21656
Group Owner: [email protected]
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-