| Message ID | 20191114220139.11138-1-saeedm@mellanox.com |
|---|---|
| State | Superseded |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [net-next,1/1] netfilter: nf_tables_offload: Fix dangling extack pointer | expand |
Hi Saeed, Thanks for your patch. On Thu, Nov 14, 2019 at 10:02:00PM +0000, Saeed Mahameed wrote: > 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. There's a fix for this in nf-next: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git/commit/?id=be193f5e21d0ec674badef9fde8eca71fb2d8546 Will send a pull request asap.
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));
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@mellanox.com> Cc: Pablo Neira Ayuso <pablo@netfilter.org> --- net/netfilter/nf_tables_offload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)