Phil Sutter <phil@xxxxxx> wrote:
> Prepare for hooks with NULL ops.dev pointer (due to non-existent device)
> and store the interface name and length as specified by the user upon
> creation. No functional change intended.
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> include/net/netfilter/nf_tables.h | 2 ++
> net/netfilter/nf_tables_api.c | 6 +++---
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index c302b396e1a7..efd6b55b4914 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -1191,6 +1191,8 @@ struct nft_hook {
> struct list_head list;
> struct nf_hook_ops ops;
> struct rcu_head rcu;
> + char ifname[IFNAMSIZ];
> + u8 ifnamelen;
> };
>
> /**
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 3ffb728309af..f1710aab5188 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2173,7 +2173,6 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
> const struct nlattr *attr)
> {
> struct net_device *dev;
> - char ifname[IFNAMSIZ];
> struct nft_hook *hook;
> int err;
>
> @@ -2183,12 +2182,13 @@ static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
> goto err_hook_alloc;
> }
>
> - nla_strscpy(ifname, attr, IFNAMSIZ);
> + nla_strscpy(hook->ifname, attr, IFNAMSIZ);
> + hook->ifnamelen = nla_len(attr);
Hmm. nft_netdev_hook_alloc has no netlink attribute policy validation
:-/
Can you add another patch that fixes this up?
I'd suggest to move the if (nla_type(tmp) != NFTA_DEVICE_NAME)
test from nf_tables_parse_netdev_hooks() into nft_netdev_hook_alloc
so nft_chain_parse_netdev() also has this test.
Then,
> - nla_strscpy(ifname, attr, IFNAMSIZ);
Into:
err = nla_strscpy(ifname, attr, IFNAMSIZ)
if (err < 0)
goto err_hook_dev;
so we validate that
a) attr is NFTA_DEVICE_NAME
b) length doesn't exceed IFNAMSIZ.
ATM this check is implicit because nla_strscpy() always
null-terminates and the next line will check that the
device with this name actually exists.
But that check is removed later.
This patch can then set
hook->ifnamelen = err
without risk that nla_len() returns 0xfff0 or some other bogus
value.
