All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tuo Li <islituo@gmail.com>
To: ayush.sawal@chelsio.com, andrew+netdev@lunn.ch,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, almasrymina@google.com, dtatulea@nvidia.com,
	jacob.e.keller@intel.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	baijiaju1990@gmail.com, Tuo Li <islituo@gmail.com>
Subject: [PATCH] chcr_ktls: fix a possible null-pointer dereference in chcr_ktls_dev_add()
Date: Thu, 31 Oct 2024 00:23:52 +1100	[thread overview]
Message-ID: <20241030132352.154488-1-islituo@gmail.com> (raw)

There is a possible null-pointer dereference related to the wait-completion
synchronization mechanism in the function chcr_ktls_dev_add().

Consider the following execution scenario:

  chcr_ktls_cpl_act_open_rpl()   //641
    u_ctx = adap->uld[CXGB4_ULD_KTLS].handle;   //686
    if (u_ctx) {  //687
    complete(&tx_info->completion);  //704

The variable u_ctx is checked by an if statement at Line 687, which means
it can be NULL. Then, complete() is called at Line 704, which will wake
up wait_for_completion_xxx().

Consider the wait_for_completion_timeout() in chcr_ktls_dev_add():

  chcr_ktls_dev_add()  //412
    u_ctx = adap->uld[CXGB4_ULD_KTLS].handle;  //432
    wait_for_completion_timeout(&tx_info->completion, 30 * HZ); //551
    xa_erase(&u_ctx->tid_list, tx_info->tid);  //580

The variable u_ctx is dereferenced without being rechecked at Line 580
after the wait_for_completion_timeout(), which can introduce a null-pointer
dereference. Besides, the variable u_ctx is also checked at Line 442 in
chcr_ktls_dev_add(), which indicates that u_ctx is likely to be NULL in
some execution contexts.

To fix this possible null-pointer dereference, a NULL check is put ahead of
the call to xa_erase().

This potential bug was discovered using an experimental static analysis
tool developed by our team. The tool deduces complete() and
wait_for_completion() pairs using alias analysis. It then applies data
flow analysis to detect null-pointer dereferences across synchronization
points.

Fixes: 65e302a9bd57 ("cxgb4/ch_ktls: Clear resources when pf4 device is removed") 
Signed-off-by: Tuo Li <islituo@gmail.com>
---
 drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index e8e460a92e0e..524c8e032bc8 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -577,7 +577,8 @@ static int chcr_ktls_dev_add(struct net_device *netdev, struct sock *sk,
 	cxgb4_remove_tid(&tx_info->adap->tids, tx_info->tx_chan,
 			 tx_info->tid, tx_info->ip_family);
 
-	xa_erase(&u_ctx->tid_list, tx_info->tid);
+	if (u_ctx)
+		xa_erase(&u_ctx->tid_list, tx_info->tid);
 
 put_module:
 	/* release module refcount */
-- 
2.43.0


             reply	other threads:[~2024-10-30 13:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-30 13:23 Tuo Li [this message]
2024-11-04 16:07 ` [PATCH] chcr_ktls: fix a possible null-pointer dereference in chcr_ktls_dev_add() Simon Horman
2024-11-04 23:32   ` Tuo Li
2024-11-08 13:34     ` Simon Horman
2024-11-14 10:30       ` Tuo Li
2024-11-08 15:00 ` Markus Elfring
2024-11-14 10:37   ` Tuo Li
2024-11-14 12:26     ` Markus Elfring
2024-11-14 14:22       ` Tuo Li
2024-11-15  9:15         ` Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241030132352.154488-1-islituo@gmail.com \
    --to=islituo@gmail.com \
    --cc=almasrymina@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=ayush.sawal@chelsio.com \
    --cc=baijiaju1990@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.