From patchwork Thu Aug 1 11:47:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hannah Peuckmann X-Patchwork-Id: 1967623 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WZRzv3rBlz1yZv for ; Thu, 1 Aug 2024 21:48:11 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1sZUHt-0007eX-1M; Thu, 01 Aug 2024 11:48:05 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1sZUHp-0007dn-W6 for kernel-team@lists.ubuntu.com; Thu, 01 Aug 2024 11:48:02 +0000 Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 97E6D3F298 for ; Thu, 1 Aug 2024 11:48:01 +0000 (UTC) Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-426624f4b7aso10647845e9.3 for ; Thu, 01 Aug 2024 04:48:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722512881; x=1723117681; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=y799nbyaCxTyTKT2zQ/OTb9gdhKhX8aPqpdlWZxn56U=; b=ZFHGSMj9203Fs6aAwfo1CsJggEeXz46vufLSwfQx4Obg4lS2yd5XqEFkCFZfEee5bx 3zAoAlqdyiye/fFXJOEUZHdLs2GifFKWrnV0bJR+LUY/4Iq0hNJSmz3D+gzLEptDShJK Aeb2OWdeoVpOB755WIewZBGXigZ4TxqX+Cux68P6aUECmp6CPJq3E6ghjmd2QIlBJRGD wWTLtNFkwBPan92U2kyiTFcIPvJia4e1DUOj18KlVscuQn+9ahf4F34XmF77G7rKBHWu QK2v8m0JXsgxN9uoPK9dan4kth4YkfGChUY3DJIJnhgMDFbsv4/0Dxk/ChgzqOHI8GUn EF1w== X-Gm-Message-State: AOJu0Yye1gQKpdlFVeBqM2gd7/l8DuSeXUE1FS1QnNssIDwMizszRrqj PKER7ovWIcMi0Yxn3hYut77lzbB3aDiwPxVyROpHTjQz7tszZkLnuoStIM9GorP1qsbWp0LRTdY a5jROdgI2qIWlnLyXBac3tyNwKRcQ4KyrLjD4DqPJ3oABlB/Vk0pCzvqmujNTuwIeXrx73K2K1m Us9tdRU9ifZ+cQ X-Received: by 2002:a05:600c:3c93:b0:428:f17:6baf with SMTP id 5b1f17b1804b1-428b4ae22f7mr11152115e9.5.1722512880570; Thu, 01 Aug 2024 04:48:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEcSGtN7YGOZhnADysEgjoQuhQ1AeU/6wWZXo1wTwV/5s5NdRnb97ngUbtbClZm0q+K44c/fw== X-Received: by 2002:a05:600c:3c93:b0:428:f17:6baf with SMTP id 5b1f17b1804b1-428b4ae22f7mr11151975e9.5.1722512879825; Thu, 01 Aug 2024 04:47:59 -0700 (PDT) Received: from bojack.home (085081016045.dynamic.telenor.dk. [85.81.16.45]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4282b8ad475sm55525375e9.13.2024.08.01.04.47.59 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Aug 2024 04:47:59 -0700 (PDT) From: Hannah Peuckmann To: kernel-team@lists.ubuntu.com Subject: [SRU][F][PATCH 1/1] netfilter: nf_tables: restore set elements when delete set fails Date: Thu, 1 Aug 2024 13:47:56 +0200 Message-ID: <20240801114757.50429-2-hannah.peuckmann@canonical.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240801114757.50429-1-hannah.peuckmann@canonical.com> References: <20240801114757.50429-1-hannah.peuckmann@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Pablo Neira Ayuso From abort path, nft_mapelem_activate() needs to restore refcounters to the original state. Currently, it uses the set->ops->walk() to iterate over these set elements. The existing set iterator skips inactive elements in the next generation, this does not work from the abort path to restore the original state since it has to skip active elements instead (not inactive ones). This patch moves the check for inactive elements to the set iterator callback, then it reverses the logic for the .activate case which needs to skip active elements. Toggle next generation bit for elements when delete set command is invoked and call nft_clear() from .activate (abort) path to restore the next generation bit. The splat below shows an object in mappings memleak: [43929.457523] ------------[ cut here ]------------ [43929.457532] WARNING: CPU: 0 PID: 1139 at include/net/netfilter/nf_tables.h:1237 nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [...] [43929.458014] RIP: 0010:nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [43929.458076] Code: 83 f8 01 77 ab 49 8d 7c 24 08 e8 37 5e d0 de 49 8b 6c 24 08 48 8d 7d 50 e8 e9 5c d0 de 8b 45 50 8d 50 ff 89 55 50 85 c0 75 86 <0f> 0b eb 82 0f 0b eb b3 0f 1f 40 00 90 90 90 90 90 90 90 90 90 90 [43929.458081] RSP: 0018:ffff888140f9f4b0 EFLAGS: 00010246 [43929.458086] RAX: 0000000000000000 RBX: ffff8881434f5288 RCX: dffffc0000000000 [43929.458090] RDX: 00000000ffffffff RSI: ffffffffa26d28a7 RDI: ffff88810ecc9550 [43929.458093] RBP: ffff88810ecc9500 R08: 0000000000000001 R09: ffffed10281f3e8f [43929.458096] R10: 0000000000000003 R11: ffff0000ffff0000 R12: ffff8881434f52a0 [43929.458100] R13: ffff888140f9f5f4 R14: ffff888151c7a800 R15: 0000000000000002 [43929.458103] FS: 00007f0c687c4740(0000) GS:ffff888390800000(0000) knlGS:0000000000000000 [43929.458107] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [43929.458111] CR2: 00007f58dbe5b008 CR3: 0000000123602005 CR4: 00000000001706f0 [43929.458114] Call Trace: [43929.458118] [43929.458121] ? __warn+0x9f/0x1a0 [43929.458127] ? nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [43929.458188] ? report_bug+0x1b1/0x1e0 [43929.458196] ? handle_bug+0x3c/0x70 [43929.458200] ? exc_invalid_op+0x17/0x40 [43929.458211] ? nft_setelem_data_deactivate+0xd7/0xf0 [nf_tables] [43929.458271] ? nft_setelem_data_deactivate+0xe4/0xf0 [nf_tables] [43929.458332] nft_mapelem_deactivate+0x24/0x30 [nf_tables] [43929.458392] nft_rhash_walk+0xdd/0x180 [nf_tables] [43929.458453] ? __pfx_nft_rhash_walk+0x10/0x10 [nf_tables] [43929.458512] ? rb_insert_color+0x2e/0x280 [43929.458520] nft_map_deactivate+0xdc/0x1e0 [nf_tables] [43929.458582] ? __pfx_nft_map_deactivate+0x10/0x10 [nf_tables] [43929.458642] ? __pfx_nft_mapelem_deactivate+0x10/0x10 [nf_tables] [43929.458701] ? __rcu_read_unlock+0x46/0x70 [43929.458709] nft_delset+0xff/0x110 [nf_tables] [43929.458769] nft_flush_table+0x16f/0x460 [nf_tables] [43929.458830] nf_tables_deltable+0x501/0x580 [nf_tables] Fixes: 628bd3e49cba ("netfilter: nf_tables: drop map element references from preparation phase") Signed-off-by: Pablo Neira Ayuso CVE-2024-27012 (cherry picked from commit e79b47a8615d42c68aaeb68971593333667382ed linux-6.9.y) Signed-off-by: Hannah Peuckmann --- net/netfilter/nf_tables_api.c | 25 +++++++++++++++++++++++++ net/netfilter/nft_set_bitmap.c | 4 +--- net/netfilter/nft_set_hash.c | 8 ++------ net/netfilter/nft_set_rbtree.c | 4 +--- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index b3209a856209..10e7d8e9df04 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -426,6 +426,12 @@ static int nft_mapelem_deactivate(const struct nft_ctx *ctx, const struct nft_set_iter *iter, struct nft_set_elem *elem) { + struct nft_set_ext *ext = nft_set_elem_ext(set, elem); + + if (!nft_set_elem_active(ext, iter->genmask)) + return 0; + + nft_set_elem_change_active(ctx->net, set, ext); nft_setelem_data_deactivate(ctx->net, set, elem); return 0; @@ -4055,6 +4061,8 @@ static int nf_tables_bind_check_setelem(const struct nft_ctx *ctx, struct nft_set_elem *elem) { const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); + if (!nft_set_elem_active(ext, iter->genmask)) + return 0; enum nft_registers dreg; dreg = nft_type_to_reg(set->dtype); @@ -4128,6 +4136,13 @@ static int nft_mapelem_activate(const struct nft_ctx *ctx, const struct nft_set_iter *iter, struct nft_set_elem *elem) { + struct nft_set_ext *ext = nft_set_elem_ext(set, elem); + + /* called from abort path, reverse check to undo changes. */ + if (nft_set_elem_active(ext, iter->genmask)) + return 0; + + nft_clear(ctx->net, ext); nft_setelem_data_activate(ctx->net, set, elem); return 0; @@ -4370,6 +4385,9 @@ static int nf_tables_dump_setelem(const struct nft_ctx *ctx, const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); struct nft_set_dump_args *args; + if (!nft_set_elem_active(ext, iter->genmask)) + return 0; + if (nft_set_elem_expired(ext) || nft_set_elem_is_dead(ext)) return 0; @@ -5228,9 +5246,13 @@ static int nft_flush_set(const struct nft_ctx *ctx, const struct nft_set_iter *iter, struct nft_set_elem *elem) { + const struct nft_set_ext *ext = nft_set_elem_ext(set, elem); struct nft_trans *trans; int err; + if (!nft_set_elem_active(ext, iter->genmask)) + return 0; + trans = nft_trans_alloc_gfp(ctx, NFT_MSG_DELSETELEM, sizeof(struct nft_trans_elem), GFP_ATOMIC); if (!trans) @@ -7860,6 +7882,9 @@ static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx, const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv); const struct nft_data *data; + if (!nft_set_elem_active(ext, iter->genmask)) + return 0; + if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) && *nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END) return 0; diff --git a/net/netfilter/nft_set_bitmap.c b/net/netfilter/nft_set_bitmap.c index b0f6b1490e1a..544835080fe2 100644 --- a/net/netfilter/nft_set_bitmap.c +++ b/net/netfilter/nft_set_bitmap.c @@ -170,7 +170,7 @@ static void nft_bitmap_activate(const struct net *net, nft_bitmap_location(set, nft_set_ext_key(&be->ext), &idx, &off); /* Enter 11 state. */ priv->bitmap[idx] |= (genmask << off); - nft_set_elem_change_active(net, set, &be->ext); + nft_clear(net, &be->ext); } static bool nft_bitmap_flush(const struct net *net, @@ -222,8 +222,6 @@ static void nft_bitmap_walk(const struct nft_ctx *ctx, list_for_each_entry_rcu(be, &priv->list, head) { if (iter->count < iter->skip) goto cont; - if (!nft_set_elem_active(&be->ext, iter->genmask)) - goto cont; elem.priv = be; diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c index 0581d5499c5a..7f49c6c37c94 100644 --- a/net/netfilter/nft_set_hash.c +++ b/net/netfilter/nft_set_hash.c @@ -191,7 +191,7 @@ static void nft_rhash_activate(const struct net *net, const struct nft_set *set, { struct nft_rhash_elem *he = elem->priv; - nft_set_elem_change_active(net, set, &he->ext); + nft_clear(net, &he->ext); } static bool nft_rhash_flush(const struct net *net, @@ -279,8 +279,6 @@ static void nft_rhash_walk(const struct nft_ctx *ctx, struct nft_set *set, if (iter->count < iter->skip) goto cont; - if (!nft_set_elem_active(&he->ext, iter->genmask)) - goto cont; elem.priv = he; @@ -573,7 +571,7 @@ static void nft_hash_activate(const struct net *net, const struct nft_set *set, { struct nft_hash_elem *he = elem->priv; - nft_set_elem_change_active(net, set, &he->ext); + nft_clear(net, &he->ext); } static bool nft_hash_flush(const struct net *net, @@ -627,8 +625,6 @@ static void nft_hash_walk(const struct nft_ctx *ctx, struct nft_set *set, hlist_for_each_entry_rcu(he, &priv->table[i], node) { if (iter->count < iter->skip) goto cont; - if (!nft_set_elem_active(&he->ext, iter->genmask)) - goto cont; elem.priv = he; diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c index f44473287079..ec322d818de0 100644 --- a/net/netfilter/nft_set_rbtree.c +++ b/net/netfilter/nft_set_rbtree.c @@ -529,7 +529,7 @@ static void nft_rbtree_activate(const struct net *net, { struct nft_rbtree_elem *rbe = elem->priv; - nft_set_elem_change_active(net, set, &rbe->ext); + nft_clear(net, &rbe->ext); } static bool nft_rbtree_flush(const struct net *net, @@ -598,8 +598,6 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx, if (iter->count < iter->skip) goto cont; - if (!nft_set_elem_active(&rbe->ext, iter->genmask)) - goto cont; elem.priv = rbe;