netdev - [net-next 1/1] netfilter: nf_tables_offload: Fix dangling extack pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191114220139.11138-1-saeedm@mellanox.com>
Date:   Thu, 14 Nov 2019 22:02:00 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>,
        "netfilter-devel@...r.kernel.org" <netfilter-devel@...r.kernel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>
Subject: [net-next 1/1] netfilter: nf_tables_offload: Fix dangling extack
 pointer

nft_flow_cls_offload_setup() will setup cls_flow->common->extack to point
to a local extack object on its stack, this extack pointer is meant to
be used on nft_setup_cb_call() driver's callbacks, which is called after
nft_flow_cls_offload_setup() is terminated and thus will lead to a
dangling pointer.

Apparently no one is going to be actually using this extack pointer to
report issues to netlink, although drivers might still write to it.

Since this is never going to be read, we can make it static to avoid
crashing.

Alternatively we can:
1. move the extack object to the callers stack.
2. set cls_flow->common->extack = NULL.

I chose this approach since it has the least impact.

This was found by a code review, i didn't encounter any actual issue nor
i tried to reproduce.

Fixes: c5d275276ff4 ("netfilter: nf_tables_offload: add nft_flow_cls_offload_setup()")
Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
Cc: Pablo Neira Ayuso <pablo@...filter.org>
---
 net/netfilter/nf_tables_offload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index cdea3010c7a0..6178b8ace3d3 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -161,7 +161,7 @@ static void nft_flow_cls_offload_setup(struct flow_cls_offload *cls_flow,
 				       const struct nft_flow_rule *flow,
 				       enum flow_cls_command command)
 {
-	struct netlink_ext_ack extack;
+	static struct netlink_ext_ack extack; /* not actually read into netlink */
 	__be16 proto = ETH_P_ALL;

 	memset(cls_flow, 0, sizeof(*cls_flow));
-- 
2.21.0