From patchwork Mon Jun 11 12:04:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taehee Yoo X-Patchwork-Id: 927630 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.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=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="J5hB3jW/"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 414BXH6kcdz9ryk for ; Mon, 11 Jun 2018 22:04:47 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932652AbeFKMEq (ORCPT ); Mon, 11 Jun 2018 08:04:46 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:34533 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932603AbeFKMEp (ORCPT ); Mon, 11 Jun 2018 08:04:45 -0400 Received: by mail-pg0-f67.google.com with SMTP id q4-v6so8595009pgr.1 for ; Mon, 11 Jun 2018 05:04:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=5pq6y61kp4UBub9jCSndQTITAz/c1M6UYTAAPkvnxpA=; b=J5hB3jW/ZLsRdYr+Sn+wTvrFAPC77bDfMn16uNH3coDRl14a0ZnwEJwAs7sc9ryCu4 7jnOCpuqT7rzD9OgQv65lbTOKqXTxVAoG/vf2urZyunxvoZYZugyBHALed2YBbibcjf9 QHNDWC5hq+K50cenvSZBWv9k3plKP/q1DGePwhR1LtmMdC8c/ixp9eaLX62hLiDITDqc zifjCacZ0Qomjvm16HQu2j0mBbYLx6K8K4IcJ3KKGie43B6HD4pu1lHPk5AA6iLCyfok RmjfsXpwG+HQL+76HJxrojZMkEXhveosH+b2lOtV53l7kfA9U6APV9Hh/N51ymUtR6iX XJUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=5pq6y61kp4UBub9jCSndQTITAz/c1M6UYTAAPkvnxpA=; b=c/RNBVzQZqT+8V21yCDza1HF5fHpyWC9wxubFBe7hkJuv6T7hOpYdc2hhJMKJZCAI8 KeoXmEBYG2Cp8mMNdM1FIy2ihg09u3jjualpS5yj7vZDkxCE404HUCzBWYPhnb2aQKxA U/tjI7pSA1fASRZ4jc8NzZrsr+ndKQBAPYwNcnnxmAh/Ph3SD8ly5LytG6MHwnbjYbTL WVOhZO2/hS4qsHlnf5r0o7j+aHU6OusCyMvExBXaEL7JxH6vDdaP8YGGGQ2FMn6XQOwX 4oJeswSBHkrptjTlO6WOIO8UHD6mc9Bhdw8CHh/TmrVp6YmfSeFSdVT9J3OLsiQVM5jd TGDw== X-Gm-Message-State: APt69E1WhW0MBYn+0RjJfmpGW7Dux2sJY/lviDArY1dnJYEXUmhZB4A7 GB97Z8ZuQitPRbwjv7OPruWNLg== X-Google-Smtp-Source: ADUXVKJFTn748Higi8+c/9oSGEV+ToE/UzhfOo0i2KsLJah3DpPA6KCmSX142DKza0mUH/cXp+AVtg== X-Received: by 2002:a63:9e0a:: with SMTP id s10-v6mr14845093pgd.305.1528718685110; Mon, 11 Jun 2018 05:04:45 -0700 (PDT) Received: from ap-To-be-filled-by-O-E-M.8.8.8.8 ([125.130.197.10]) by smtp.gmail.com with ESMTPSA id z19-v6sm8724886pfe.163.2018.06.11.05.04.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Jun 2018 05:04:44 -0700 (PDT) From: Taehee Yoo To: pablo@netfilter.org, netfilter-devel@vger.kernel.org Cc: ap420073@gmail.com Subject: [PATCH nf-next] netfilter: nf_tables: fix jumpstack depth validation Date: Mon, 11 Jun 2018 21:04:39 +0900 Message-Id: <20180611120439.2744-1-ap420073@gmail.com> X-Mailer: git-send-email 2.9.3 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org The level of struct nft_ctx is updated by nf_tables_check_loops(). That is used to validate jumpstack depth. But jumpstack validation routine doesn't update and validate recursively. So, in some cases, chain depth can be bigger than the NFT_JUMP_STACK_SIZE. After this patch, The jumpstack validation routine is located in the nft_chain_validate(). When new rules or new set elements are added, the nft_table_validate() is called by the nf_tables_newrule and the nf_tables_newsetelem. The nft_table_validate() calls the nft_chain_validate() that visit all their children chains recursively. So it can update depth of chain certainly. Reproducer: %cat ./test.sh #!/bin/bash nft add table ip filter nft add chain ip filter input { type filter hook input priority 0\; } for ((i=0;i<20;i++)); do nft add chain ip filter a$i done nft add rule ip filter input jump a1 for ((i=0;i<10;i++)); do nft add rule ip filter a$i jump a$((i+1)) done for ((i=11;i<19;i++)); do nft add rule ip filter a$i jump a$((i+1)) done nft add rule ip filter a10 jump a11 Result: [ 168.803743] kernel BUG at net/netfilter/nf_tables_core.c:186! [ 168.810881] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI [ 168.812091] Modules linked in: nf_tables nfnetlink ip_tables x_tables [ 168.812091] CPU: 0 PID: 8 Comm: ksoftirqd/0 Not tainted 4.17.0-rc7+ #186 [ 168.812091] RIP: 0010:nft_do_chain+0x9fe/0xf50 [nf_tables] [ 168.812091] RSP: 0000:ffff88011a5475b0 EFLAGS: 00010212 [ 168.812091] RAX: 00000000fffffffd RBX: ffff88011a5476a0 RCX: 0000000000000000 [ 168.812091] RDX: 0000000000000010 RSI: ffff880111d69bd8 RDI: ffff88011a5476b0 [ 168.812091] RBP: ffff88011a547870 R08: ffffed00234a8ed6 R09: ffffed00234a8ed5 [ 168.812091] R10: ffff88011a5476af R11: ffffed00234a8ed6 R12: ffff88011a5478b8 [ 168.881353] R13: ffff880111d69be0 R14: ffff880111d69bc0 R15: dffffc0000000000 [ 168.887792] FS: 0000000000000000(0000) GS:ffff88011b600000(0000) knlGS:0000000000000000 [ 168.887792] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 168.905313] CR2: 00007ffd58ee6148 CR3: 0000000067416000 CR4: 00000000001006f0 [ 168.917438] Call Trace: [ 168.917438] ? __save_stack_trace+0x73/0xd0 [ 168.922459] ? __nft_trace_packet+0x1a0/0x1a0 [nf_tables] [ 168.922459] ? save_stack+0x92/0xa0 [ 168.922459] ? ip_rcv+0x802/0xe10 [ 168.922459] ? sched_clock_cpu+0x144/0x180 [ 168.922459] ? sched_clock_local+0xe2/0x150 [ 168.922459] ? __lock_acquire+0xcea/0x4ed0 [ 168.922459] ? sched_clock_cpu+0x144/0x180 [ 168.922459] ? debug_check_no_locks_freed+0x280/0x280 [ 168.922459] ? nft_do_chain_ipv4+0x16f/0x1e0 [nf_tables] [ 168.922459] nft_do_chain_ipv4+0x16f/0x1e0 [nf_tables] [ 168.922459] ? nft_do_chain_arp+0xa0/0xa0 [nf_tables] [ 168.922459] ? lock_acquire+0x193/0x380 [ 168.922459] ? lock_acquire+0x193/0x380 [ 168.922459] ? ip_local_deliver+0x1c6/0x3c0 [ 168.922459] nf_hook_slow+0xae/0x170 [ 168.922459] ip_local_deliver+0x293/0x3c0 [ 168.922459] ? ip_call_ra_chain+0x490/0x490 [ 168.922459] ? ip_rcv_finish+0x1910/0x1910 [ 168.922459] ip_rcv+0x802/0xe10 [ ... ] Signed-off-by: Taehee Yoo --- include/net/netfilter/nf_tables.h | 4 ++-- net/netfilter/nf_tables_api.c | 10 +++------- net/netfilter/nft_immediate.c | 3 +++ net/netfilter/nft_lookup.c | 13 +++++++++++-- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 08c005c..a7d6476 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -150,6 +150,7 @@ static inline void nft_data_debug(const struct nft_data *data) * @portid: netlink portID of the original message * @seq: netlink sequence number * @family: protocol family + * @level: depth of the chains * @report: notify via unicast netlink message */ struct nft_ctx { @@ -160,6 +161,7 @@ struct nft_ctx { u32 portid; u32 seq; u8 family; + u8 level; bool report; }; @@ -865,7 +867,6 @@ enum nft_chain_flags { * @table: table that this chain belongs to * @handle: chain handle * @use: number of jump references to this chain - * @level: length of longest path to this chain * @flags: bitmask of enum nft_chain_flags * @name: name of the chain */ @@ -878,7 +879,6 @@ struct nft_chain { struct nft_table *table; u64 handle; u32 use; - u16 level; u8 flags:6, genmask:2; char *name; diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index ca4c4d9..b7d8c25 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -2384,6 +2384,9 @@ int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain) struct nft_rule *rule; int err; + if (ctx->level == NFT_JUMP_STACK_SIZE) + return -EMLINK; + list_for_each_entry(rule, &chain->rules, list) { if (!nft_is_active_next(ctx->net, rule)) continue; @@ -6826,13 +6829,6 @@ int nft_validate_register_store(const struct nft_ctx *ctx, err = nf_tables_check_loops(ctx, data->verdict.chain); if (err < 0) return err; - - if (ctx->chain->level + 1 > - data->verdict.chain->level) { - if (ctx->chain->level + 1 == NFT_JUMP_STACK_SIZE) - return -EMLINK; - data->verdict.chain->level = ctx->chain->level + 1; - } } return 0; diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c index 15adf8c..9f0542e 100644 --- a/net/netfilter/nft_immediate.c +++ b/net/netfilter/nft_immediate.c @@ -99,6 +99,7 @@ static int nft_immediate_validate(const struct nft_ctx *ctx, { const struct nft_immediate_expr *priv = nft_expr_priv(expr); const struct nft_data *data; + struct nft_ctx *pctx = (struct nft_ctx *)ctx; int err; if (priv->dreg != NFT_REG_VERDICT) @@ -109,9 +110,11 @@ static int nft_immediate_validate(const struct nft_ctx *ctx, switch (data->verdict.code) { case NFT_JUMP: case NFT_GOTO: + pctx->level++; err = nft_chain_validate(ctx, data->verdict.chain); if (err < 0) return err; + pctx->level--; break; default: break; diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c index 42e6fad..af13093 100644 --- a/net/netfilter/nft_lookup.c +++ b/net/netfilter/nft_lookup.c @@ -156,6 +156,8 @@ static int nft_lookup_validate_setelem(const struct nft_ctx *ctx, { const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); const struct nft_data *data; + struct nft_ctx *pctx = (struct nft_ctx *)ctx; + int err; if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) && *nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END) @@ -165,10 +167,17 @@ static int nft_lookup_validate_setelem(const struct nft_ctx *ctx, switch (data->verdict.code) { case NFT_JUMP: case NFT_GOTO: - return nft_chain_validate(ctx, data->verdict.chain); + pctx->level++; + err = nft_chain_validate(ctx, data->verdict.chain); + if (err < 0) + return err; + pctx->level--; + break; default: - return 0; + break; } + + return 0; } static int nft_lookup_validate(const struct nft_ctx *ctx,