[ovs-dev] [branch-2.9] Revert "flow: Fix buffer overread for crafted IPv6 packets."

Darrell Ball dball at vmware.com
Mon Jul 16 14:57:59 UTC 2018


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, "ovs-dev-bounces at openvswitch.org on behalf of Lucas Alvares Gomes" <ovs-dev-bounces at openvswitch.org on behalf of lucasagomes at gmail.com> 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&amp;data=02%7C01%7Cdball%40vmware.com%7Cd5698b8b3b84410609fc08d5eb210c5a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636673449543755379&amp;sdata=0s1hShgwC7nXh21U9hWXg%2FLbtgO3pgUczJQkodKzzw0%3D&amp;reserved=0
    (the link will eventually expire)
    
    Acked-By: Lucas Alvares Gomes <lucasagomes at gmail.com>
    
    On Sat, Jul 14, 2018 at 9:33 PM Justin Pettit <jpettit at ovn.org> 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 <blp at ovn.org>
    > Fixes: 0760bd61a66 ("flow: Fix buffer overread for crafted IPv6 packets.")
    > Signed-off-by: Justin Pettit <jpettit at ovn.org>
    >
    > ---
    > 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
    > dev at openvswitch.org
    > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=02%7C01%7Cdball%40vmware.com%7Cd5698b8b3b84410609fc08d5eb210c5a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636673449543755379&amp;sdata=%2FyB6MWLpdYQa2cplcpeMGQakdPAXgS0zHakvZ95gSBY%3D&amp;reserved=0
    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=02%7C01%7Cdball%40vmware.com%7Cd5698b8b3b84410609fc08d5eb210c5a%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636673449543755379&amp;sdata=%2FyB6MWLpdYQa2cplcpeMGQakdPAXgS0zHakvZ95gSBY%3D&amp;reserved=0
    



More information about the dev mailing list