| From 8ca6ec23f9139028538d814d6c41dfa8122ad305 Mon Sep 17 00:00:00 2001 |
| From: Florian Westphal <fw@strlen.de> |
| Date: Thu, 5 Mar 2020 11:15:36 +0100 |
| Subject: [PATCH] netfilter: nf_tables: fix infinite loop when expr is not |
| available |
| |
| commit 1d305ba40eb8081ff21eeb8ca6ba5c70fd920934 upstream. |
| |
| nft will loop forever if the kernel doesn't support an expression: |
| |
| 1. nft_expr_type_get() appends the family specific name to the module list. |
| 2. -EAGAIN is returned to nfnetlink, nfnetlink calls abort path. |
| 3. abort path sets ->done to true and calls request_module for the |
| expression. |
| 4. nfnetlink replays the batch, we end up in nft_expr_type_get() again. |
| 5. nft_expr_type_get attempts to append family-specific name. This |
| one already exists on the list, so we continue |
| 6. nft_expr_type_get adds the generic expression name to the module |
| list. -EAGAIN is returned, nfnetlink calls abort path. |
| 7. abort path encounters the family-specific expression which |
| has 'done' set, so it gets removed. |
| 8. abort path requests the generic expression name, sets done to true. |
| 9. batch is replayed. |
| |
| If the expression could not be loaded, then we will end up back at 1), |
| because the family-specific name got removed and the cycle starts again. |
| |
| Note that userspace can SIGKILL the nft process to stop the cycle, but |
| the desired behaviour is to return an error after the generic expr name |
| fails to load the expression. |
| |
| Fixes: eb014de4fd418 ("netfilter: nf_tables: autoload modules from the abort path") |
| Signed-off-by: Florian Westphal <fw@strlen.de> |
| Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c |
| index 503734ba81b8..4933b96fadf1 100644 |
| --- a/net/netfilter/nf_tables_api.c |
| +++ b/net/netfilter/nf_tables_api.c |
| @@ -6836,13 +6836,8 @@ static void nf_tables_module_autoload(struct net *net) |
| list_splice_init(&net->nft.module_list, &module_list); |
| mutex_unlock(&net->nft.commit_mutex); |
| list_for_each_entry_safe(req, next, &module_list, list) { |
| - if (req->done) { |
| - list_del(&req->list); |
| - kfree(req); |
| - } else { |
| - request_module("%s", req->module); |
| - req->done = true; |
| - } |
| + request_module("%s", req->module); |
| + req->done = true; |
| } |
| mutex_lock(&net->nft.commit_mutex); |
| list_splice(&module_list, &net->nft.module_list); |
| @@ -7620,6 +7615,7 @@ static void __net_exit nf_tables_exit_net(struct net *net) |
| __nft_release_tables(net); |
| mutex_unlock(&net->nft.commit_mutex); |
| WARN_ON_ONCE(!list_empty(&net->nft.tables)); |
| + WARN_ON_ONCE(!list_empty(&net->nft.module_list)); |
| } |
| |
| static struct pernet_operations nf_tables_net_ops = { |
| -- |
| 2.7.4 |
| |