Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 17 Mar 2020 12:18:37 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Amitkumar Karwar <amitkarwar@gmail.com>,
        Ganapathi Bhat <ganapathi.bhat@nxp.com>
Cc:     linux-wireless@vger.kernel.org
Subject: [bug report] wireless: mwifiex: initial commit for Marvell mwifiex
 driver
Message-ID: <20200317091837.GA18001@mwanda>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

[ This is old, but maybe the driver is still really actively maintained
  so maybe someone knows the answer.  - dan ]

Hello Marvell Developers,

The patch 5e6e3a92b9a4: "wireless: mwifiex: initial commit for
Marvell mwifiex driver" from Mar 21, 2011, leads to the following
static checker warning:

	drivers/net/wireless/marvell/mwifiex/11n.c:505 mwifiex_11n_delete_tx_ba_stream_tbl_entry()
	error: we previously assumed 'tx_ba_tsr_tbl' could be null (see line 498)

drivers/net/wireless/marvell/mwifiex/11n.c
   472  /*
   473   * This function checks if the given pointer is valid entry of
   474   * Tx BA Stream table.
   475   */
   476  static int mwifiex_is_tx_ba_stream_ptr_valid(struct mwifiex_private *priv,
   477                                  struct mwifiex_tx_ba_stream_tbl *tx_tbl_ptr)
                                                                         ^^^^^^^^^^
This is always NULL.

   478  {
   479          struct mwifiex_tx_ba_stream_tbl *tx_ba_tsr_tbl;
   480  
   481          list_for_each_entry(tx_ba_tsr_tbl, &priv->tx_ba_stream_tbl_ptr, list) {
   482                  if (tx_ba_tsr_tbl == tx_tbl_ptr)
                            ^^^^^^^^^^^^^
tx_ba_tsr_tbl is the list iterator, which is never NULL so this will
never return true.

   483                          return true;
   484          }
   485  
   486          return false;
   487  }
   488  
   489  /*
   490   * This function deletes the given entry in Tx BA Stream table.
   491   *
   492   * The function also performs a validity check on the supplied
   493   * pointer before trying to delete.
   494   */
   495  void mwifiex_11n_delete_tx_ba_stream_tbl_entry(struct mwifiex_private *priv,
   496                                  struct mwifiex_tx_ba_stream_tbl *tx_ba_tsr_tbl)
   497  {
   498          if (!tx_ba_tsr_tbl &&
                     ^^^^^^^^^^^^^
Check for NULL

   499              mwifiex_is_tx_ba_stream_ptr_valid(priv, tx_ba_tsr_tbl))
                                                            ^^^^^^^^^^^^^
Which is passed to here.  So maybe the NULL check is reversed?

   500                  return;
   501  
   502          mwifiex_dbg(priv->adapter, INFO,
   503                      "info: tx_ba_tsr_tbl %p\n", tx_ba_tsr_tbl);
   504  
   505          list_del(&tx_ba_tsr_tbl->list);
                          ^^^^^^^^^^^^^^^^^^^
Unchecked NULL dereference

   506  
   507          kfree(tx_ba_tsr_tbl);
   508  }

regards,
dan carpenter