{"id":812661,"url":"http://patchwork.ozlabs.org/api/patches/812661/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/patch/20170911233332.7594-4-xiyou.wangcong@gmail.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":"<20170911233332.7594-4-xiyou.wangcong@gmail.com>","list_archive_url":null,"date":"2017-09-11T23:33:32","name":"[net,v3,3/3] net_sched: carefully handle tcf_block_put()","commit_ref":null,"pull_url":null,"state":"accepted","archived":true,"hash":"5f0e9b3a3fca735a442b1494e94f5cd358b6d8d9","submitter":{"id":211,"url":"http://patchwork.ozlabs.org/api/people/211/?format=json","name":"Cong Wang","email":"xiyou.wangcong@gmail.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/20170911233332.7594-4-xiyou.wangcong@gmail.com/mbox/","series":[{"id":2596,"url":"http://patchwork.ozlabs.org/api/series/2596/?format=json","web_url":"http://patchwork.ozlabs.org/project/netdev/list/?series=2596","date":"2017-09-11T23:33:29","name":"net_sched: fix filter chain reference counting","version":3,"mbox":"http://patchwork.ozlabs.org/series/2596/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/812661/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/812661/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=gmail.com header.i=@gmail.com\n\theader.b=\"ksbl6rlf\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xrklW6KF9z9s8J\n\tfor ;\n\tTue, 12 Sep 2017 09:33:59 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751251AbdIKXd6 (ORCPT );\n\tMon, 11 Sep 2017 19:33:58 -0400","from mail-pg0-f67.google.com ([74.125.83.67]:37905 \"EHLO\n\tmail-pg0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750957AbdIKXdy (ORCPT\n\t); Mon, 11 Sep 2017 19:33:54 -0400","by mail-pg0-f67.google.com with SMTP id m30so361833pgn.5\n\tfor ; Mon, 11 Sep 2017 16:33:54 -0700 (PDT)","from tw-172-25-30-113.office.twttr.net ([8.25.197.25])\n\tby smtp.gmail.com with ESMTPSA id\n\tf5sm15676526pga.30.2017.09.11.16.33.52\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 11 Sep 2017 16:33:52 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=t57XK8bxT+MO5YW9uD+UENXeUq/5y272bm4sEPgpBNs=;\n\tb=ksbl6rlf5/dmfzqAgs1PQsf+2qX3D7pV0/LecPLN8H8F7IL9/rx+W9OHJelu5o/2tF\n\tO8f4/HCRnSaWQYC/1q2W/vpqFjG7XXV8OufnpPLUbdkNUS2KV+1U53JHhGJ9z6mC/Cij\n\tPmTGonH8cH+J0i+TqsObRI7Edmn+9Retr9M0Qna4eRCtsIU8spb4UupMVr1cZuEfYC2w\n\tHt3joO5PTIoMBrTPXlmgf9FPwDvzoVVmr/I1e2lE84hiOjqx01yseUdap9r2RYtAMP8p\n\tI8g/T/3yb59DYRW9p+RwjWrkInwbJCJY2U6uIOiDIFIxb6bZuTP8zcOh2zlikFR975RL\n\tu+BA==","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=t57XK8bxT+MO5YW9uD+UENXeUq/5y272bm4sEPgpBNs=;\n\tb=gVG1dwEoybAIegJqOgDIm0KqD3DDnKyqRZ26Z5Y2ASXW4vIhDjDozfOEHBz6n+zB3s\n\t+XvzVPwkb3e089/MvZqC65Dv7A9BLLzjGoU6rL1GISwTppIQxEnT4DzxkNNvo9kMigSS\n\tSOaXAqgTKWiGQIU9pzXx1wfLE0gFqDqI40VLxochp4MiizB6mr794kMYJ32ILzacVuYz\n\thSA5tTVBSdGn9jqEFpP8yTvcxdTB1xkTYXVeGS6fVJFXkFW2Grpq3qSNxoQl3rJRa6B8\n\tyu/irf/oLRslW4YDfwpgqblYv26GD3bGYylPX2+7ewNHPLBIrVta7mnMmeNqOJf2ZJGV\n\tP1jA==","X-Gm-Message-State":"AHPjjUjM0Q7+Zy6PIZhAUi2r2emjOYLRGO5BmNg5CQmd3hETO+iV75Qs\n\taJMAcJEiMsEXP48JFug=","X-Google-Smtp-Source":"ADKCNb6+zYR4cZkXYxpBKGS/gGdBsusmEm+rKq+UTH0+62hOFJqEqLPTdm7eaI9rSsmS9DFEKve6pg==","X-Received":"by 10.84.224.75 with SMTP id a11mr15133377plt.106.1505172833774; \n\tMon, 11 Sep 2017 16:33:53 -0700 (PDT)","From":"Cong Wang ","To":"netdev@vger.kernel.org","Cc":"jiri@mellanox.com, jakub.kicinski@netronome.com, jhs@mojatatu.com,\n\tCong Wang ","Subject":"[Patch net v3 3/3] net_sched: carefully handle tcf_block_put()","Date":"Mon, 11 Sep 2017 16:33:32 -0700","Message-Id":"<20170911233332.7594-4-xiyou.wangcong@gmail.com>","X-Mailer":"git-send-email 2.9.4","In-Reply-To":"<20170911233332.7594-1-xiyou.wangcong@gmail.com>","References":"<20170911233332.7594-1-xiyou.wangcong@gmail.com>","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"},"content":"As pointed out by Jiri, there is still a race condition between\ntcf_block_put() and tcf_chain_destroy() in a RCU callback. There\nis no way to make it correct without proper locking or synchronization,\nbecause both operate on a shared list.\n\nLocking is hard, because the only lock we can pick here is a spinlock,\nhowever, in tc_dump_tfilter() we iterate this list with a sleeping\nfunction called (tcf_chain_dump()), which makes using a lock to protect\nchain_list almost impossible.\n\nJiri suggested the idea of holding a refcnt before flushing, this works\nbecause it guarantees us there would be no parallel tcf_chain_destroy()\nduring the loop, therefore the race condition is gone. But we have to\nbe very careful with proper synchronization with RCU callbacks.\n\nSuggested-by: Jiri Pirko \nCc: Jamal Hadi Salim \nSigned-off-by: Cong Wang \n---\n net/sched/cls_api.c | 24 ++++++++++++++++++------\n 1 file changed, 18 insertions(+), 6 deletions(-)","diff":"diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c\nindex d29e79d98a69..0b2219adf520 100644\n--- a/net/sched/cls_api.c\n+++ b/net/sched/cls_api.c\n@@ -275,15 +275,27 @@ void tcf_block_put(struct tcf_block *block)\n \n \t/* XXX: Standalone actions are not allowed to jump to any chain, and\n \t * bound actions should be all removed after flushing. However,\n-\t * filters are destroyed in RCU callbacks, we have to flush and wait\n-\t * for them inside the loop, otherwise we race with RCU callbacks on\n-\t * this list.\n+\t * filters are destroyed in RCU callbacks, we have to hold the chains\n+\t * first, otherwise we would always race with RCU callbacks on this list\n+\t * without proper locking.\n \t */\n-\tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list) {\n+\n+\t/* Wait for existing RCU callbacks to cool down. */\n+\trcu_barrier();\n+\n+\t/* Hold a refcnt for all chains, except 0, in case they are gone. */\n+\tlist_for_each_entry(chain, &block->chain_list, list)\n+\t\tif (chain->index)\n+\t\t\ttcf_chain_hold(chain);\n+\n+\t/* No race on the list, because no chain could be destroyed. */\n+\tlist_for_each_entry(chain, &block->chain_list, list)\n \t\ttcf_chain_flush(chain);\n-\t\trcu_barrier();\n-\t}\n \n+\t/* Wait for RCU callbacks to release the reference count. */\n+\trcu_barrier();\n+\n+\t/* At this point, all the chains should have refcnt == 1. */\n \tlist_for_each_entry_safe(chain, tmp, &block->chain_list, list)\n \t\ttcf_chain_put(chain);\n \tkfree(block);\n","prefixes":["net","v3","3/3"]}