Re: [vpp-dev] A bug in IP reassembly?
Ok. I'll find some time tomorrow to push a patch fixing both v4 and v6.
-Original Message-
From: Klement Sekera
Sent: Tuesday, September 25, 2018 6:02 PM
To: Kingwel Xie ; 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 (, 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]
-=-=-=-=-=-=-=-=-=-=-=-
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 (, 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: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[vpp-dev] A bug in IP reassembly?
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 (, 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 (#10638): https://lists.fd.io/g/vpp-dev/message/10638
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]
-=-=-=-=-=-=-=-=-=-=-=-
