Re: [PATCH nf] netfilter: nf_tables: check for valid chain type pointer before dereference
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2020-01-18 20:31:04
Also in:
netfilter-devel
On Thu, Jan 16, 2020 at 10:11:09PM +0100, Florian Westphal wrote:
quoted hunk ↗ jump to hunk
Its possible to create tables in a family that isn't supported/known. Then, when adding a base chain, the table pointer can be NULL. This gets us a NULL ptr dereference in nf_tables_addchain(). Fixes: baae3e62f31618 ("netfilter: nf_tables: fix chain type module reference handling") Reported-by: syzbot+156a04714799b1d480bc@syzkaller.appspotmail.com Signed-off-by: Florian Westphal <fw@strlen.de> --- net/netfilter/nf_tables_api.c | 8 ++++++++ 1 file changed, 8 insertions(+)diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 65f51a2e9c2a..e8976128cdb1 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c@@ -953,6 +953,9 @@ static int nf_tables_newtable(struct net *net, struct sock *nlsk, struct nft_ctx ctx; int err; + if (family >= NFPROTO_NUMPROTO) + return -EAFNOSUPPORT; + lockdep_assert_held(&net->nft.commit_mutex); attr = nla[NFTA_TABLE_NAME]; table = nft_table_lookup(net, attr, family, genmask);@@ -1765,6 +1768,9 @@ static int nft_chain_parse_hook(struct net *net, ha[NFTA_HOOK_PRIORITY] == NULL) return -EINVAL; + if (family >= NFPROTO_NUMPROTO) + return -EAFNOSUPPORT; + hook->num = ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM])); hook->priority = ntohl(nla_get_be32(ha[NFTA_HOOK_PRIORITY]));@@ -1774,6 +1780,8 @@ static int nft_chain_parse_hook(struct net *net, family, autoload); if (IS_ERR(type)) return PTR_ERR(type); + } else if (!type) { + return -EOPNOTSUPP;
I think this check should be enough. I mean, NFPROTO_NUMPROTO still allows for creating tables for families that don't exist (<= NFPROTO_NUMPROTO) and why bother on creating such table. As long as such table does not crash the kernel, I think it's fine. No changes can be attached anymore anyway. Otherwise, if a helper function to check for the families that are really supported could be another alternative. But not sure it is worth? Let me know, thanks.