On Wed, Oct 25, 2023 at 10:08:26PM +0200, Phil Sutter wrote:
> In theory, dumpreset may fail and invalidate the preceeding log message.
> Fix this and use the occasion to prepare for object reset locking, which
> benefits from a few unrelated changes:
>
> * Add an early call to nfnetlink_unicast if not resetting which
> effectively skips the audit logging but also unindents it.
> * Extract the table's name from the netlink attribute (which is verified
> via earlier table lookup) to not rely upon validity of the looked up
> table pointer.
> * Do not use local variable family, it will vanish.
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> net/netfilter/nf_tables_api.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 3c1fd8283bf4..d0f7274b7ffe 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -7767,6 +7767,7 @@ static int nf_tables_dump_obj_done(struct netlink_callback *cb)
> static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
> const struct nlattr * const nla[])
> {
> + const struct nftables_pernet *nft_net = nft_pernet(info->net);
> struct netlink_ext_ack *extack = info->extack;
> u8 genmask = nft_genmask_cur(info->net);
> u8 family = info->nfmsg->nfgen_family;
> @@ -7776,6 +7777,7 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
> struct sk_buff *skb2;
> bool reset = false;
> u32 objtype;
> + char *buf;
> int err;
>
> if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
> @@ -7814,27 +7816,23 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
> if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET)
> reset = true;
>
> - if (reset) {
> - const struct nftables_pernet *nft_net;
> - char *buf;
> -
> - nft_net = nft_pernet(net);
> - buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, nft_net->base_seq);
> -
> - audit_log_nfcfg(buf,
> - family,
> - 1,
> - AUDIT_NFT_OP_OBJ_RESET,
> - GFP_ATOMIC);
> - kfree(buf);
> - }
> -
> err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid,
> info->nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0,
> family, table, obj, reset);
> if (err < 0)
> goto err_fill_obj_info;
>
> + if (!reset)
> + return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);
More simple with?
if (reset) {
buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
nla_len(nla[NFTA_OBJ_TABLE]),
(char *)nla_data(nla[NFTA_OBJ_TABLE]),
nft_net->base_seq);
audit_log_nfcfg(buf, info->nfmsg->nfgen_family,
1, AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
kfree(buf);
}
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);
single call to nfnetlink_unicast().
> +
> + buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
> + nla_len(nla[NFTA_OBJ_TABLE]),
> + (char *)nla_data(nla[NFTA_OBJ_TABLE]),
> + nft_net->base_seq);
> + audit_log_nfcfg(buf, info->nfmsg->nfgen_family, 1,
> + AUDIT_NFT_OP_OBJ_RESET, GFP_ATOMIC);
> + kfree(buf);
> +
> return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);
>
> err_fill_obj_info:
> --
> 2.41.0
>
