From patchwork Mon Nov 20 21:34:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roman Kapl X-Patchwork-Id: 839786 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3yghp13pgkz9t2R for ; Tue, 21 Nov 2017 08:35:05 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753116AbdKTVfC (ORCPT ); Mon, 20 Nov 2017 16:35:02 -0500 Received: from droplet.rkapl.cz ([46.101.253.207]:45990 "EHLO droplet.rkapl.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752882AbdKTVfC (ORCPT ); Mon, 20 Nov 2017 16:35:02 -0500 Received: from localhost.localdomain (dynamic-2a00-1028-8380-0f86-0000-0000-0000-0002.ipv6.broadband.iol.cz [IPv6:2a00:1028:8380:f86::2]) by droplet.rkapl.cz (Postfix) with ESMTPA id ADD37848C4; Mon, 20 Nov 2017 22:35:00 +0100 (CET) From: Roman Kapl To: netdev@vger.kernel.org Cc: Roman Kapl Subject: [PATCH] net: sched: crash on blocks with goto chain action Date: Mon, 20 Nov 2017 22:34:54 +0100 Message-Id: <20171119161716.5865-1-code@rkapl.cz> X-Mailer: git-send-email 2.15.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org tcf_block_put_ext has assumed that all filters (and thus their goto actions) are destroyed in RCU callback and thus can not race with our list iteration. However, that is not true during netns cleanup (see tcf_exts_get_net comment). Prevent the user after free by holding the current list element we are iterating over (foreach_safe is not enough). To reproduce, run the following in a netns and then delete the ns: ip link add dtest type dummy tc qdisc add dev dtest ingress tc filter add dev dtest chain 1 parent ffff: handle 1 prio 1 flower action goto chain 2 Signed-off-by: Roman Kapl --- The mail was original rejected by vger, this is a re-send to netdev@vger only (with the same message ID). Sorry for any confusion. --- net/sched/cls_api.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 7d97f612c9b9..58fed2ea3379 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -344,16 +344,26 @@ static void tcf_block_put_final(struct work_struct *work) } /* XXX: Standalone actions are not allowed to jump to any chain, and bound - * actions should be all removed after flushing. However, filters are now - * destroyed in tc filter workqueue with RTNL lock, they can not race here. + * actions should be all removed after flushing. */ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q, struct tcf_block_ext_info *ei) { - struct tcf_chain *chain, *tmp; + struct tcf_chain *chain, *last = NULL; - list_for_each_entry_safe(chain, tmp, &block->chain_list, list) + list_for_each_entry(chain, &block->chain_list, list) { + if (last) + tcf_chain_put(last); + /* Flushing a chain may release any other chain in this block, + * so we have to hold on to the chain across flush to known + * which one comes next. + */ + tcf_chain_hold(chain); tcf_chain_flush(chain); + last = chain; + } + if (last) + tcf_chain_put(last); tcf_block_offload_unbind(block, q, ei);