It looks like the code flow ordering was changed recently by
62b0859dd89d(flow: Introduce IP packet sanity checks).
- if (OVS_UNLIKELY(size < sizeof *nh)) {
+ if (OVS_UNLIKELY(!ipv6_sanity_check(nh, size))) {
goto out;
}
- nh = data_pull(&data, &size, sizeof *nh);
+ data_pull(&data, &size, sizeof *nh);
plen = ntohs(nh->ip6_plen);
- if (OVS_UNLIKELY(plen > size)) {
- goto out;
- }
- /* Jumbo Payload option not supported yet. */
- if (OVS_UNLIKELY(size - plen > UINT8_MAX)) {
- goto out;
- }
On 7/16/18, 6:35 AM, "[email protected] on behalf of Lucas
Alvares Gomes" <[email protected] on behalf of
[email protected]> wrote:
Thanks Justin,
In networking-ovn (the OVN driver for OpenStack Neutron) we are seem
an IPv6 related test failure right now [0] and I can confirm that
after I've applied this patch locally and re-ran the test it works
again.
[0]
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flogs.openstack.org%2F59%2F570459%2F2%2Fcheck%2Fnetworking-ovn-tempest-dsvm-ovs-release%2F4b5bb1d%2Flogs%2Ftestr_results.html.gz&data=02%7C01%7Cdball%40vmware.com%7Cd5698b8b3b84410609fc08d5eb210c5a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636673449543755379&sdata=0s1hShgwC7nXh21U9hWXg%2FLbtgO3pgUczJQkodKzzw0%3D&reserved=0
(the link will eventually expire)
Acked-By: Lucas Alvares Gomes <[email protected]>
On Sat, Jul 14, 2018 at 9:33 PM Justin Pettit <[email protected]> wrote:
>
> This reverts commit 0760bd61a666e9fa866fcb5ed67f48f34895d2f6.
>
> This patch was a cherry-pick from a bug fix in the master branch that
> fixed an overread for IPv6 packets. However, the backport introduced a
> problem in older branches, since the code path is different. In the
> master branch, this check is done on the raw packet data, which starts
> at the beginning of the IPv6 packet. In older branches, this check is
> done after a call to data_pull(), which subtracts the IPv6 header length
> from the 'size' variable. This means that valid IPv6 packets aren't
> being processed since the check thinks they are too long.
>
> CC: Ben Pfaff <[email protected]>
> Fixes: 0760bd61a66 ("flow: Fix buffer overread for crafted IPv6 packets.")
> Signed-off-by: Justin Pettit <[email protected]>
>
> ---
> This patch should be backported to older branches starting with
branch-2.9.
> ---
> lib/flow.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/flow.c b/lib/flow.c
> index c78f46d6c15a..f9d7c2a74007 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -804,7 +804,7 @@ miniflow_extract(struct dp_packet *packet, struct
miniflow *dst)
> nh = data_pull(&data, &size, sizeof *nh);
>
> plen = ntohs(nh->ip6_plen);
> - if (OVS_UNLIKELY(plen + IPV6_HEADER_LEN > size)) {
> + if (OVS_UNLIKELY(plen > size)) {
> goto out;
> }
> /* Jumbo Payload option not supported yet. */
> --
> 2.17.1
>
> _______________________________________________
> dev mailing list
> [email protected]
>
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=02%7C01%7Cdball%40vmware.com%7Cd5698b8b3b84410609fc08d5eb210c5a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636673449543755379&sdata=%2FyB6MWLpdYQa2cplcpeMGQakdPAXgS0zHakvZ95gSBY%3D&reserved=0
_______________________________________________
dev mailing list
[email protected]
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=02%7C01%7Cdball%40vmware.com%7Cd5698b8b3b84410609fc08d5eb210c5a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636673449543755379&sdata=%2FyB6MWLpdYQa2cplcpeMGQakdPAXgS0zHakvZ95gSBY%3D&reserved=0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
