{"id":811001,"url":"http://patchwork.ozlabs.org/api/patches/811001/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/patch/150478759820.28665.14031878598812204399.stgit@firesoul/","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":"<150478759820.28665.14031878598812204399.stgit@firesoul>","list_archive_url":null,"date":"2017-09-07T12:33:18","name":"[V2,net-next,2/2] xdp: catch invalid XDP_REDIRECT API usage","commit_ref":null,"pull_url":null,"state":"deferred","archived":true,"hash":"6623e90a4afab3137abbc66100ecc195c0ffa927","submitter":{"id":13625,"url":"http://patchwork.ozlabs.org/api/people/13625/?format=json","name":"Jesper Dangaard Brouer","email":"brouer@redhat.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/150478759820.28665.14031878598812204399.stgit@firesoul/mbox/","series":[{"id":1995,"url":"http://patchwork.ozlabs.org/api/series/1995/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/list/?series=1995","date":"2017-09-07T12:33:08","name":"Fixes for XDP_REDIRECT map","version":2,"mbox":"http://patchwork.ozlabs.org/series/1995/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/811001/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/811001/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=)","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx08.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=brouer@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xp0H95ZYYz9s81\n\tfor ;\n\tThu, 7 Sep 2017 22:33:25 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S932122AbdIGMdX (ORCPT );\n\tThu, 7 Sep 2017 08:33:23 -0400","from mx1.redhat.com ([209.132.183.28]:55862 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S932105AbdIGMdV (ORCPT );\n\tThu, 7 Sep 2017 08:33:21 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 8A2EBC0587D6;\n\tThu, 7 Sep 2017 12:33:21 +0000 (UTC)","from firesoul.localdomain (ovpn-200-42.brq.redhat.com\n\t[10.40.200.42])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 24F7818B11;\n\tThu, 7 Sep 2017 12:33:19 +0000 (UTC)","from [192.168.5.1] (localhost [IPv6:::1])\n\tby firesoul.localdomain (Postfix) with ESMTP id 49ECE3073EC87;\n\tThu, 7 Sep 2017 14:33:18 +0200 (CEST)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 8A2EBC0587D6","Subject":"[V2 PATCH net-next 2/2] xdp: catch invalid XDP_REDIRECT API usage","From":"Jesper Dangaard Brouer ","To":"netdev@vger.kernel.org, \"David S. Miller\" ","Cc":"Daniel Borkmann ,\n\tJohn Fastabend ,\n\tAndy Gospodarek ,\n\tJesper Dangaard Brouer ","Date":"Thu, 07 Sep 2017 14:33:18 +0200","Message-ID":"<150478759820.28665.14031878598812204399.stgit@firesoul>","In-Reply-To":"<150478756604.28665.6915020425359475729.stgit@firesoul>","References":"<150478756604.28665.6915020425359475729.stgit@firesoul>","User-Agent":"StGit/0.17.1-dirty","MIME-Version":"1.0","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.32]);\n\tThu, 07 Sep 2017 12:33:21 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"},"content":"Catch different invalid XDP_REDIRECT and bpf_redirect_map API usage.\n\nIt is fairly easy to create a dangling redirect_info->map pointer,\nwhich (until John or Daniel fix this) can crash the kernel.\n\nThe intended usage of the BPF helper bpf_redirect_map(), is to return\nXDP_REDIRECT action after invoking it, but there is nothing stopping\nthe bpf_prog to return anything else. When XDP_REDIRECT isn't\nreturned, then a dangling ->map pointer is left behind, as\nxdp_do_redirect() isn't called.\n\nThis also happens for drivers not implementing XDP_REDIRECT, as they\nare not aware of this new XDP_REDIRECT return code, they leave the map\npointer dangling.\n\nThe simply solution to check for a dangling ->map pointer after each\ndriver napi->poll() invocation, see xdp_do_map_check().\n\nThis patch also add a check for a dangling ->map_to_flush pointer.\nThis should be considered a driver bug, as the driver contract is that\na pair of xdp_do_redirect and xdp_do_flush_map MUST be called in the\nsame cpu context.\n\nNote, we need to check after each drivers napi->poll call, as:\n 1. DevA poll call bpf_redirect_map() but not xdp_do_redirect()\n 2. DevB bpf_prog uses bpf_redirect() and call xdp_do_redirect()\n which now use map from DevA\n\nSigned-off-by: Jesper Dangaard Brouer \n---\n include/linux/filter.h | 1 +\n net/core/dev.c | 3 +++\n net/core/filter.c | 25 +++++++++++++++++++++++++\n 3 files changed, 29 insertions(+)","diff":"diff --git a/include/linux/filter.h b/include/linux/filter.h\nindex d29e58fde364..0c48941e0022 100644\n--- a/include/linux/filter.h\n+++ b/include/linux/filter.h\n@@ -724,6 +724,7 @@ int xdp_do_redirect(struct net_device *dev,\n \t\t struct xdp_buff *xdp,\n \t\t struct bpf_prog *prog);\n void xdp_do_flush_map(void);\n+void xdp_do_map_check(struct napi_struct *napi);\n \n void bpf_warn_invalid_xdp_action(u32 act);\n void bpf_warn_invalid_xdp_redirect(u32 ifindex);\ndiff --git a/net/core/dev.c b/net/core/dev.c\nindex 6f845e4fec17..7eac642b469f 100644\n--- a/net/core/dev.c\n+++ b/net/core/dev.c\n@@ -5320,6 +5320,7 @@ static void busy_poll_stop(struct napi_struct *napi, void *have_poll_lock)\n \t */\n \trc = napi->poll(napi, BUSY_POLL_BUDGET);\n \ttrace_napi_poll(napi, rc, BUSY_POLL_BUDGET);\n+\txdp_do_map_check(napi);\n \tnetpoll_poll_unlock(have_poll_lock);\n \tif (rc == BUSY_POLL_BUDGET)\n \t\t__napi_schedule(napi);\n@@ -5367,6 +5368,7 @@ void napi_busy_loop(unsigned int napi_id,\n \t\t}\n \t\twork = napi_poll(napi, BUSY_POLL_BUDGET);\n \t\ttrace_napi_poll(napi, work, BUSY_POLL_BUDGET);\n+\t\txdp_do_map_check(napi);\n count:\n \t\tif (work > 0)\n \t\t\t__NET_ADD_STATS(dev_net(napi->dev),\n@@ -5529,6 +5531,7 @@ static int napi_poll(struct napi_struct *n, struct list_head *repoll)\n \tif (test_bit(NAPI_STATE_SCHED, &n->state)) {\n \t\twork = n->poll(n, weight);\n \t\ttrace_napi_poll(n, work, weight);\n+\t\txdp_do_map_check(n);\n \t}\n \n \tWARN_ON_ONCE(work > weight);\ndiff --git a/net/core/filter.c b/net/core/filter.c\nindex 3767470cab6c..f0e1135eeb9d 100644\n--- a/net/core/filter.c\n+++ b/net/core/filter.c\n@@ -2500,6 +2500,31 @@ void xdp_do_flush_map(void)\n }\n EXPORT_SYMBOL_GPL(xdp_do_flush_map);\n \n+void xdp_do_map_check(struct napi_struct *napi)\n+{\n+\tstruct redirect_info *ri = this_cpu_ptr(&redirect_info);\n+\n+\t/* XDP drivers (and XDP-generic) must invoke xdp_do_redirect()\n+\t * when bpf_prog use helper bpf_redirect_map(), else the map\n+\t * pointer can be left dangling. Catch this invalid API\n+\t * usage, instead of potentially crashing.\n+\t */\n+\tif (ri->map) {\n+\t\tri->map = NULL;\n+\t\tnet_err_ratelimited(\"%s: caught invalid XDP bpf_redirect_map\\n\",\n+\t\t\t\t napi->dev->name);\n+\t\ttrace_xdp_exception(napi->dev, NULL, XDP_REDIRECT);\n+\t}\n+\tif (ri->map_to_flush) { /* Driver bug */\n+\t\tnet_err_ratelimited(\"%s: XDP driver miss xdp_do_flush_map\\n\",\n+\t\t\t\t napi->dev->name);\n+\t\ttrace_xdp_exception(napi->dev, NULL, XDP_REDIRECT);\n+\t\t/* Flush map, else pkts can be stuck on XDP TXq */\n+\t\txdp_do_flush_map();\n+\t}\n+}\n+EXPORT_SYMBOL_GPL(xdp_do_map_check);\n+\n static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,\n \t\t\t struct bpf_prog *xdp_prog)\n {\n","prefixes":["V2","net-next","2/2"]}