linux-kernel - Re: [PATCH] net: Initialize ctx to avoid memory allocation error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250313201007.GA26103@breakpoint.cc>
Date: Thu, 13 Mar 2025 21:10:07 +0100
From: Florian Westphal <fw@...len.de>
To: Chenyuan Yang <chenyuan0y@...il.com>
Cc: netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, casey@...aufler-ca.com
Subject: Re: [PATCH] net: Initialize ctx to avoid memory allocation error

[ trim CCs, CC Casey ]

Chenyuan Yang <chenyuan0y@...il.com> wrote:
> It is possible that ctx in nfqnl_build_packet_message() could be used
> before it is properly initialize, which is only initialized
> by nfqnl_get_sk_secctx().
> 
> This patch corrects this problem by initializing the lsmctx to a safe
> value when it is declared.
> 
> This is similar to the commit 35fcac7a7c25
> ("audit: Initialize lsmctx to avoid memory allocation error").

Fixes: 2d470c778120 ("lsm: replace context+len with lsm_context")

> Signed-off-by: Chenyuan Yang <chenyuan0y@...il.com>
> ---
>  net/netfilter/nfnetlink_queue.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 5c913987901a..8b7b39d8a109 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -567,7 +567,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
>  	enum ip_conntrack_info ctinfo = 0;
>  	const struct nfnl_ct_hook *nfnl_ct;
>  	bool csum_verify;
> -	struct lsm_context ctx;
> +	struct lsm_context ctx = { NULL, 0, 0 };
>  	int seclen = 0;
>  	ktime_t tstamp;

Someone that understands LSM should clarify what seclen == 0 means.

seclen needs to be > 0 or no secinfo is passed to userland,
yet the secctx release function is called anyway.

Should seclen be initialised to -1?  Or we need the change below too?

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -812,7 +812,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
        }

        nlh->nlmsg_len = skb->len;
-       if (seclen >= 0)
+       if (seclen > 0)
                security_release_secctx(&ctx);
        return skb;

@@ -821,7 +821,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
        kfree_skb(skb);
        net_err_ratelimited("nf_queue: error creating packet message\n");
 nlmsg_failure:
-       if (seclen >= 0)
+       if (seclen > 0)
                security_release_secctx(&ctx);
        return NULL;
 }