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

[Florian Westphal]

[ trim CCs, CC Casey ]

Chenyuan Yang <chenyuan0y@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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;
 }