{"id":805983,"url":"http://patchwork.ozlabs.org/api/patches/805983/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/patch/1503682263-17858-2-git-send-email-pieter.jansenvanvuuren@netronome.com/","project":{"id":7,"url":"http://patchwork.ozlabs.org/api/projects/7/?format=json","name":"Linux network development","link_name":"netdev","list_id":"netdev.vger.kernel.org","list_email":"netdev@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null,"list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<1503682263-17858-2-git-send-email-pieter.jansenvanvuuren@netronome.com>","list_archive_url":null,"date":"2017-08-25T17:31:01","name":"[net,1/3] nfp: fix unchecked flow dissector use","commit_ref":null,"pull_url":null,"state":"accepted","archived":true,"hash":"c5fe41522f1075fa6bb26976cd20aa4000d3143e","submitter":{"id":72232,"url":"http://patchwork.ozlabs.org/api/people/72232/?format=json","name":"Pieter Jansen van Vuuren","email":"pieter.jansenvanvuuren@netronome.com"},"delegate":{"id":34,"url":"http://patchwork.ozlabs.org/api/users/34/?format=json","username":"davem","first_name":"David","last_name":"Miller","email":"davem@davemloft.net"},"mbox":"http://patchwork.ozlabs.org/project/netdev/patch/1503682263-17858-2-git-send-email-pieter.jansenvanvuuren@netronome.com/mbox/","series":[],"comments":"http://patchwork.ozlabs.org/api/patches/805983/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/805983/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"dTQarolY\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xf7Vw0dqlz9sN5\n\tfor ;\n\tSat, 26 Aug 2017 03:31:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1757346AbdHYRbS (ORCPT );\n\tFri, 25 Aug 2017 13:31:18 -0400","from mail-wm0-f52.google.com ([74.125.82.52]:37160 \"EHLO\n\tmail-wm0-f52.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1757329AbdHYRbN (ORCPT\n\t); Fri, 25 Aug 2017 13:31:13 -0400","by mail-wm0-f52.google.com with SMTP id b189so3451869wmd.0\n\tfor ; Fri, 25 Aug 2017 10:31:12 -0700 (PDT)","from pieter-Netronome.netronome.com\n\t(host-79-78-33-110.static.as9105.net. [79.78.33.110])\n\tby smtp.gmail.com with ESMTPSA id\n\t40sm7418994wrz.8.2017.08.25.10.31.10\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);\n\tFri, 25 Aug 2017 10:31:11 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=rF/smWrKCeh2wVPuWQ4txHwUMdqNaNddxdtmpKe5l2I=;\n\tb=dTQarolYJmKzvYKD91lQypqk1YB4zwDKALpRwKCi/c7VM5MG5p0G7cBaeVnli/uzUs\n\tn7xQfjLESZIVtNzWhWqv+FGAL3Ue9foJEZFoM5g5QVsuKu/MkMYpQlF54B45BHGXo9KF\n\tcGi0eyMduMlstTQ2h5YqDQvunjMP2w8cSKQ9oNetY9hqm642HY+p3+xgg9mCQhOh3Hlw\n\tKtoe6653JDJd7sQztt5s95b8AglHjpTIrad3OacH2xMfhPRKbjD+rHuwR1fxmJLm1BZA\n\twxWBAewfQrZylqQtuFDOehWQtT7KEf3xqKzNJoQr9uOQVPweG+cA3vkVcL3TjU6vE0Nx\n\tOOgw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references;\n\tbh=rF/smWrKCeh2wVPuWQ4txHwUMdqNaNddxdtmpKe5l2I=;\n\tb=TkismZTM/efFLWw90l5dXtZoSusbk05FYOWpvMH141AhVdQz6bvUTWcPyCer8qq3SE\n\tEPbybIGcv3E1do9xePHrzqSIeDzjaSdWifxlZf0iV50qIEm505HfrgWbAod8bw7hpvSW\n\taxycJu+Yh7oUaPuisjexld+X/g+407cMuHMFVbYGPrJ4lUA5sSxflN3hBEEQegwAQyc/\n\t5Ui9kGdV9nPlS48z3j6hmCURrbSTDuNbvbQN57inrO0UCBJjvlpeswonRxHa8BkQ0nej\n\tU9kFoL41M4Yoh1HpZqdU8EwUiuO0xX5g/PqIzICpko4QdhCrTcXmgmk7rHQ1ErAZBR4f\n\tPItQ==","X-Gm-Message-State":"AHYfb5jCuVNbSWDnMD9y3xUyWXE5ekr5l3Ecigunouqj0uqW8Sn6CVyn\n\tWB+sAEyUO/RC6bY/TRqI8w==","X-Received":"by 10.28.16.211 with SMTP id 202mr111292wmq.62.1503682271786;\n\tFri, 25 Aug 2017 10:31:11 -0700 (PDT)","From":"Pieter Jansen van Vuuren ","To":"davem@davemloft.net","Cc":"netdev@vger.kernel.org, oss-drivers@netronome.com,\n\tsimon.horman@netronome.com, jakub.kicinski@netronome.com,\n\tPieter Jansen van Vuuren ","Subject":"[PATCH net 1/3] nfp: fix unchecked flow dissector use","Date":"Fri, 25 Aug 2017 19:31:01 +0200","Message-Id":"<1503682263-17858-2-git-send-email-pieter.jansenvanvuuren@netronome.com>","X-Mailer":"git-send-email 1.9.1","In-Reply-To":"<1503682263-17858-1-git-send-email-pieter.jansenvanvuuren@netronome.com>","References":"<1503682263-17858-1-git-send-email-pieter.jansenvanvuuren@netronome.com>","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"},"content":"Previously flow dissectors were referenced without first checking that\nthey are in use and correctly populated by TC. This patch fixes this by\nchecking each flow dissector key before referencing them.\n\nFixes: 5571e8c9f241 (\"nfp: extend flower matching capabilities\")\nSigned-off-by: Pieter Jansen van Vuuren \nReviewed-by: Jakub Kicinski \nReviewed-by: Simon Horman \n---\n drivers/net/ethernet/netronome/nfp/flower/match.c | 133 +++++++++++----------\n .../net/ethernet/netronome/nfp/flower/offload.c | 41 ++++---\n 2 files changed, 93 insertions(+), 81 deletions(-)","diff":"diff --git a/drivers/net/ethernet/netronome/nfp/flower/match.c b/drivers/net/ethernet/netronome/nfp/flower/match.c\nindex 0e08404..b365110 100644\n--- a/drivers/net/ethernet/netronome/nfp/flower/match.c\n+++ b/drivers/net/ethernet/netronome/nfp/flower/match.c\n@@ -45,6 +45,7 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame,\n \tstruct flow_dissector_key_vlan *flow_vlan;\n \tu16 tmp_tci;\n \n+\tmemset(frame, 0, sizeof(struct nfp_flower_meta_two));\n \t/* Populate the metadata frame. */\n \tframe->nfp_flow_key_layer = key_type;\n \tframe->mask_id = ~0;\n@@ -54,21 +55,20 @@ nfp_flower_compile_meta_tci(struct nfp_flower_meta_two *frame,\n \t\treturn;\n \t}\n \n-\tflow_vlan = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_VLAN,\n-\t\t\t\t\t flow->key);\n-\n-\t/* Populate the tci field. */\n-\tif (!flow_vlan->vlan_id) {\n-\t\ttmp_tci = 0;\n-\t} else {\n-\t\ttmp_tci = FIELD_PREP(NFP_FLOWER_MASK_VLAN_PRIO,\n-\t\t\t\t flow_vlan->vlan_priority) |\n-\t\t\t FIELD_PREP(NFP_FLOWER_MASK_VLAN_VID,\n-\t\t\t\t flow_vlan->vlan_id) |\n-\t\t\t NFP_FLOWER_MASK_VLAN_CFI;\n+\tif (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_VLAN)) {\n+\t\tflow_vlan = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_VLAN,\n+\t\t\t\t\t\t flow->key);\n+\t\t/* Populate the tci field. */\n+\t\tif (flow_vlan->vlan_id) {\n+\t\t\ttmp_tci = FIELD_PREP(NFP_FLOWER_MASK_VLAN_PRIO,\n+\t\t\t\t\t flow_vlan->vlan_priority) |\n+\t\t\t\t FIELD_PREP(NFP_FLOWER_MASK_VLAN_VID,\n+\t\t\t\t\t flow_vlan->vlan_id) |\n+\t\t\t\t NFP_FLOWER_MASK_VLAN_CFI;\n+\t\t\tframe->tci = cpu_to_be16(tmp_tci);\n+\t\t}\n \t}\n-\tframe->tci = cpu_to_be16(tmp_tci);\n }\n \n static void\n@@ -99,17 +99,18 @@ nfp_flower_compile_mac(struct nfp_flower_mac_mpls *frame,\n \t\t bool mask_version)\n {\n \tstruct fl_flow_key *target = mask_version ? flow->mask : flow->key;\n-\tstruct flow_dissector_key_eth_addrs *flow_mac;\n-\n-\tflow_mac = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_ETH_ADDRS,\n-\t\t\t\t\t target);\n+\tstruct flow_dissector_key_eth_addrs *addr;\n \n \tmemset(frame, 0, sizeof(struct nfp_flower_mac_mpls));\n \n-\t/* Populate mac frame. */\n-\tether_addr_copy(frame->mac_dst, &flow_mac->dst[0]);\n-\tether_addr_copy(frame->mac_src, &flow_mac->src[0]);\n+\tif (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) {\n+\t\taddr = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_ETH_ADDRS,\n+\t\t\t\t\t\t target);\n+\t\t/* Populate mac frame. */\n+\t\tether_addr_copy(frame->mac_dst, &addr->dst[0]);\n+\t\tether_addr_copy(frame->mac_src, &addr->src[0]);\n+\t}\n \n \tif (mask_version)\n \t\tframe->mpls_lse = cpu_to_be32(~0);\n@@ -121,14 +122,17 @@ nfp_flower_compile_tport(struct nfp_flower_tp_ports *frame,\n \t\t\t bool mask_version)\n {\n \tstruct fl_flow_key *target = mask_version ? flow->mask : flow->key;\n-\tstruct flow_dissector_key_ports *flow_tp;\n+\tstruct flow_dissector_key_ports *tp;\n \n-\tflow_tp = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_PORTS,\n-\t\t\t\t\t target);\n+\tmemset(frame, 0, sizeof(struct nfp_flower_tp_ports));\n \n-\tframe->port_src = flow_tp->src;\n-\tframe->port_dst = flow_tp->dst;\n+\tif (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_PORTS)) {\n+\t\ttp = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t FLOW_DISSECTOR_KEY_PORTS,\n+\t\t\t\t\t target);\n+\t\tframe->port_src = tp->src;\n+\t\tframe->port_dst = tp->dst;\n+\t}\n }\n \n static void\n@@ -137,25 +141,27 @@ nfp_flower_compile_ipv4(struct nfp_flower_ipv4 *frame,\n \t\t\tbool mask_version)\n {\n \tstruct fl_flow_key *target = mask_version ? flow->mask : flow->key;\n-\tstruct flow_dissector_key_ipv4_addrs *flow_ipv4;\n-\tstruct flow_dissector_key_basic *flow_basic;\n-\n-\tflow_ipv4 = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_IPV4_ADDRS,\n-\t\t\t\t\t target);\n+\tstruct flow_dissector_key_ipv4_addrs *addr;\n+\tstruct flow_dissector_key_basic *basic;\n \n-\tflow_basic = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n-\t\t\t\t\t target);\n-\n-\t/* Populate IPv4 frame. */\n-\tframe->reserved = 0;\n-\tframe->ipv4_src = flow_ipv4->src;\n-\tframe->ipv4_dst = flow_ipv4->dst;\n-\tframe->proto = flow_basic->ip_proto;\n \t/* Wildcard TOS/TTL for now. */\n-\tframe->tos = 0;\n-\tframe->ttl = 0;\n+\tmemset(frame, 0, sizeof(struct nfp_flower_ipv4));\n+\n+\tif (dissector_uses_key(flow->dissector,\n+\t\t\t FLOW_DISSECTOR_KEY_IPV4_ADDRS)) {\n+\t\taddr = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_IPV4_ADDRS,\n+\t\t\t\t\t\t target);\n+\t\tframe->ipv4_src = addr->src;\n+\t\tframe->ipv4_dst = addr->dst;\n+\t}\n+\n+\tif (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) {\n+\t\tbasic = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n+\t\t\t\t\t\t target);\n+\t\tframe->proto = basic->ip_proto;\n+\t}\n }\n \n static void\n@@ -164,26 +170,27 @@ nfp_flower_compile_ipv6(struct nfp_flower_ipv6 *frame,\n \t\t\tbool mask_version)\n {\n \tstruct fl_flow_key *target = mask_version ? flow->mask : flow->key;\n-\tstruct flow_dissector_key_ipv6_addrs *flow_ipv6;\n-\tstruct flow_dissector_key_basic *flow_basic;\n-\n-\tflow_ipv6 = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_IPV6_ADDRS,\n-\t\t\t\t\t target);\n-\n-\tflow_basic = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n-\t\t\t\t\t target);\n+\tstruct flow_dissector_key_ipv6_addrs *addr;\n+\tstruct flow_dissector_key_basic *basic;\n \n-\t/* Populate IPv6 frame. */\n-\tframe->reserved = 0;\n-\tframe->ipv6_src = flow_ipv6->src;\n-\tframe->ipv6_dst = flow_ipv6->dst;\n-\tframe->proto = flow_basic->ip_proto;\n \t/* Wildcard LABEL/TOS/TTL for now. */\n-\tframe->ipv6_flow_label_exthdr = 0;\n-\tframe->tos = 0;\n-\tframe->ttl = 0;\n+\tmemset(frame, 0, sizeof(struct nfp_flower_ipv6));\n+\n+\tif (dissector_uses_key(flow->dissector,\n+\t\t\t FLOW_DISSECTOR_KEY_IPV6_ADDRS)) {\n+\t\taddr = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_IPV6_ADDRS,\n+\t\t\t\t\t\t target);\n+\t\tframe->ipv6_src = addr->src;\n+\t\tframe->ipv6_dst = addr->dst;\n+\t}\n+\n+\tif (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) {\n+\t\tbasic = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n+\t\t\t\t\t\t target);\n+\t\tframe->proto = basic->ip_proto;\n+\t}\n }\n \n int nfp_flower_compile_flow_match(struct tc_cls_flower_offload *flow,\ndiff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c\nindex 4ad10bd..6c8ecc2 100644\n--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c\n+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c\n@@ -105,35 +105,40 @@ static int\n nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,\n \t\t\t\tstruct tc_cls_flower_offload *flow)\n {\n-\tstruct flow_dissector_key_control *mask_enc_ctl;\n-\tstruct flow_dissector_key_basic *mask_basic;\n-\tstruct flow_dissector_key_basic *key_basic;\n+\tstruct flow_dissector_key_basic *mask_basic = NULL;\n+\tstruct flow_dissector_key_basic *key_basic = NULL;\n \tu32 key_layer_two;\n \tu8 key_layer;\n \tint key_size;\n \n-\tmask_enc_ctl = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t\t FLOW_DISSECTOR_KEY_ENC_CONTROL,\n-\t\t\t\t\t\t flow->mask);\n+\tif (dissector_uses_key(flow->dissector,\n+\t\t\t FLOW_DISSECTOR_KEY_ENC_CONTROL)) {\n+\t\tstruct flow_dissector_key_control *mask_enc_ctl =\n+\t\t\tskb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_ENC_CONTROL,\n+\t\t\t\t\t\t flow->mask);\n+\t\t/* We are expecting a tunnel. For now we ignore offloading. */\n+\t\tif (mask_enc_ctl->addr_type)\n+\t\t\treturn -EOPNOTSUPP;\n+\t}\n+\n+\tif (dissector_uses_key(flow->dissector, FLOW_DISSECTOR_KEY_BASIC)) {\n+\t\tmask_basic = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n+\t\t\t\t\t\t flow->mask);\n \n-\tmask_basic = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n-\t\t\t\t\t flow->mask);\n+\t\tkey_basic = skb_flow_dissector_target(flow->dissector,\n+\t\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n+\t\t\t\t\t\t flow->key);\n+\t}\n \n-\tkey_basic = skb_flow_dissector_target(flow->dissector,\n-\t\t\t\t\t FLOW_DISSECTOR_KEY_BASIC,\n-\t\t\t\t\t flow->key);\n \tkey_layer_two = 0;\n \tkey_layer = NFP_FLOWER_LAYER_PORT | NFP_FLOWER_LAYER_MAC;\n \tkey_size = sizeof(struct nfp_flower_meta_one) +\n \t\t sizeof(struct nfp_flower_in_port) +\n \t\t sizeof(struct nfp_flower_mac_mpls);\n \n-\t/* We are expecting a tunnel. For now we ignore offloading. */\n-\tif (mask_enc_ctl->addr_type)\n-\t\treturn -EOPNOTSUPP;\n-\n-\tif (mask_basic->n_proto) {\n+\tif (mask_basic && mask_basic->n_proto) {\n \t\t/* Ethernet type is present in the key. */\n \t\tswitch (key_basic->n_proto) {\n \t\tcase cpu_to_be16(ETH_P_IP):\n@@ -166,7 +171,7 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,\n \t\t}\n \t}\n \n-\tif (mask_basic->ip_proto) {\n+\tif (mask_basic && mask_basic->ip_proto) {\n \t\t/* Ethernet type is present in the key. */\n \t\tswitch (key_basic->ip_proto) {\n \t\tcase IPPROTO_TCP:\n","prefixes":["net","1/3"]}