re: ath10k: implement updating shared htt txq state

re: ath10k: implement updating shared htt txq state

[Dan Carpenter]

Hello Michal Kazior,

This is a semi-automatic email about new static checker warnings.

The patch c1a43d9720d8: "ath10k: implement updating shared htt txq 
state" from Mar 6, 2016, leads to the following Smatch complaint:

drivers/net/wireless/ath/ath10k/htt_tx.c:70 __ath10k_htt_tx_txq_recalc()
	 warn: variable dereferenced before check 'txq->sta' (see line 52)

drivers/net/wireless/ath/ath10k/htt_tx.c
    51		struct ath10k *ar = hw->priv;
    52		struct ath10k_sta *arsta = (void *)txq->sta->drv_priv;
                                                   ^^^^^^^^^^
New dererence.

    53		struct ath10k_vif *arvif = (void *)txq->vif->drv_priv;
    54		unsigned long frame_cnt;
    55		unsigned long byte_cnt;
    56		int idx;
    57		u32 bit;
    58		u16 peer_id;
    59		u8 tid;
    60		u8 count;
    61	
    62		lockdep_assert_held(&ar->htt.tx_lock);
    63	
    64		if (!ar->htt.tx_q_state.enabled)
    65			return;
    66	
    67		if (ar->htt.tx_q_state.mode != HTT_TX_MODE_SWITCH_PUSH_PULL)
    68			return;
    69	
    70		if (txq->sta)
                    ^^^^^^^^
New check.

    71			peer_id = arsta->peer_id;
    72		else

There are several new warnings.  See also:

drivers/net/wireless/ath/ath10k/txrx.c:86 ath10k_txrx_tx_unref() warn: variable dereferenced before check 'txq' (see line 84)
drivers/net/wireless/ath/ath10k/mac.c:3629 ath10k_mac_txq_init() warn: variable dereferenced before check 'txq' (see line 3627)
drivers/net/wireless/ath/ath10k/mac.c:3642 ath10k_mac_txq_unref() warn: variable dereferenced before check 'txq' (see line 3637)

regards,
dan carpenter

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: ath10k: implement updating shared htt txq state

[Michal Kazior]

On 11 April 2016 at 09:12, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Michal Kazior,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch c1a43d9720d8: "ath10k: implement updating shared htt txq
> state" from Mar 6, 2016, leads to the following Smatch complaint:
>
> drivers/net/wireless/ath/ath10k/htt_tx.c:70 __ath10k_htt_tx_txq_recalc()
>          warn: variable dereferenced before check 'txq->sta' (see line 52)
>
> drivers/net/wireless/ath/ath10k/htt_tx.c
>     51          struct ath10k *ar = hw->priv;
>     52          struct ath10k_sta *arsta = (void *)txq->sta->drv_priv;
>                                                    ^^^^^^^^^^
> New dererence.
>
>     53          struct ath10k_vif *arvif = (void *)txq->vif->drv_priv;
>     54          unsigned long frame_cnt;
>     55          unsigned long byte_cnt;
>     56          int idx;
>     57          u32 bit;
>     58          u16 peer_id;
>     59          u8 tid;
>     60          u8 count;
>     61
>     62          lockdep_assert_held(&ar->htt.tx_lock);
>     63
>     64          if (!ar->htt.tx_q_state.enabled)
>     65                  return;
>     66
>     67          if (ar->htt.tx_q_state.mode != HTT_TX_MODE_SWITCH_PUSH_PULL)
>     68                  return;
>     69
>     70          if (txq->sta)
>                     ^^^^^^^^
> New check.
>
>     71                  peer_id = arsta->peer_id;
>     72          else
>
> There are several new warnings.  See also:

This is a false positive.

The `txq->sta->drv_priv` is a dynamic array which is basically equal
to `(void *)txq->sta + offsetof(struct ieee80211_sta, drv_priv)`.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: ath10k: implement updating shared htt txq state

[Dan Carpenter]

On Mon, Apr 11, 2016 at 09:33:43AM +0200, Michal Kazior wrote:
> > There are several new warnings.  See also:
> 
> This is a false positive.
> 
> The `txq->sta->drv_priv` is a dynamic array which is basically equal
> to `(void *)txq->sta + offsetof(struct ieee80211_sta, drv_priv)`.

Ah, yeah.  You're right.  The other warning should be fixed though.

regards,
dan carpenter


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: ath10k: implement updating shared htt txq state

[Dan Carpenter]

On Mon, Apr 11, 2016 at 10:44:01AM +0300, Dan Carpenter wrote:
> On Mon, Apr 11, 2016 at 09:33:43AM +0200, Michal Kazior wrote:
> > > There are several new warnings.  See also:
> > 
> > This is a false positive.
> > 
> > The `txq->sta->drv_priv` is a dynamic array which is basically equal
> > to `(void *)txq->sta + offsetof(struct ieee80211_sta, drv_priv)`.
> 
> Ah, yeah.  You're right.  The other warning should be fixed though.

Actually they're all false positives too.  Sorry about that.  Sort of
ugly and avoidable though...

regards,
dan carpenter


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k