[PATCH] netfilter: nf_tables: Add a null pointer check in two functions

[PATCH] netfilter: nf_tables: Add a null pointer check in two functions

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 23 Jan 2024 14:28:31 +0100

The result from a call of the function “kasprintf” was passed to
a subsequent function call without checking for a null pointer before
(according to a memory allocation failure).
This issue was detected by using the Coccinelle software.

Thus add a null pointer check and a jump target in affected functions.

Fixes: 8877393029e76 ("netfilter: nf_tables: Open-code audit log call in nf_tables_getrule()")
Fixes: 0854db2aaef3f ("netfilter: nf_tables: use net_generic infra for transaction data")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/netfilter/nf_tables_api.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8438a8922e4a..cb61c7a39a76 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3728,10 +3728,13 @@ static int nf_tables_getrule_reset(struct sk_buff *skb,
 			nla_len(nla[NFTA_RULE_TABLE]),
 			(char *)nla_data(nla[NFTA_RULE_TABLE]),
 			nft_net->base_seq);
+	if (!buf)
+		goto exit;
+
 	audit_log_nfcfg(buf, info->nfmsg->nfgen_family, 1,
 			AUDIT_NFT_OP_RULE_RESET, GFP_ATOMIC);
 	kfree(buf);
-
+exit:
 	return nfnetlink_unicast(skb2, net, portid);
 }

@@ -7917,6 +7920,8 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,

 		nft_net = nft_pernet(net);
 		buf = kasprintf(GFP_ATOMIC, "%s:%u", table->name, nft_net->base_seq);
+		if (!buf)
+			goto fill_obj_info;

 		audit_log_nfcfg(buf,
 				family,
@@ -7925,7 +7930,7 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
 				GFP_ATOMIC);
 		kfree(buf);
 	}
-
+fill_obj_info:
 	err = nf_tables_fill_obj_info(skb2, net, NETLINK_CB(skb).portid,
 				      info->nlh->nlmsg_seq, NFT_MSG_NEWOBJ, 0,
 				      family, table, obj, reset);
--
2.43.0

Re: [PATCH] netfilter: nf_tables: Add a null pointer check in two functions

[Phil Sutter]

Hi Markus,

On Tue, Jan 23, 2024 at 02:45:12PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 23 Jan 2024 14:28:31 +0100
> 
> The result from a call of the function “kasprintf” was passed to
> a subsequent function call without checking for a null pointer before
> (according to a memory allocation failure).
> This issue was detected by using the Coccinelle software.

This is correct and I'm fine with the patch if it avoids ringing alarm
bells somewhere, yet it doesn't fix an actual issue here since the
allocated buffer is merely passed to vsnprintf() which detects and
sanitizes %s args being NULL.

Cheers, Phil

Re: netfilter: nf_tables: Add a null pointer check in two functions

[Markus Elfring]

>> The result from a call of the function “kasprintf” was passed to
>> a subsequent function call without checking for a null pointer before
>> (according to a memory allocation failure).
>> This issue was detected by using the Coccinelle software.
>
> This is correct and I'm fine with the patch if it avoids ringing alarm
> bells somewhere, yet it doesn't fix an actual issue here since the
> allocated buffer is merely passed to vsnprintf() which detects and
> sanitizes %s args being NULL.

Should null pointer tolerance be better indicated for such use cases
by any additional means?

Regards,
Markus

Re: [PATCH] netfilter: nf_tables: Add a null pointer check in two functions

[Simon Horman]

On Tue, Jan 23, 2024 at 02:45:12PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 23 Jan 2024 14:28:31 +0100
> 
> The result from a call of the function “kasprintf” was passed to
> a subsequent function call without checking for a null pointer before
> (according to a memory allocation failure).
> This issue was detected by using the Coccinelle software.
> 
> Thus add a null pointer check and a jump target in affected functions.
> 
> Fixes: 8877393029e76 ("netfilter: nf_tables: Open-code audit log call in nf_tables_getrule()")
> Fixes: 0854db2aaef3f ("netfilter: nf_tables: use net_generic infra for transaction data")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

For the record:

Markus has posed patches of this nature many times over the past weeks
and months. They are not fixing bugs - there is no user visible problem.
And moreover they move the code away from the preferred style -
avoiding unnecessary checks around code that can handle NULL values.

Markus, please stop posting patches of this nature.