Dan Carpenter <dan.carpen...@oracle.com> writes:

> Hello Toke Høiland-Jørgensen,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch bb42f2d13ffc: "mac80211: Move reorder-sensitive TX handlers 
> to after TXQ dequeue" from Sep 22, 2016, leads to the following 
> Smatch complaint:
>
> net/mac80211/tx.c:3242 ieee80211_xmit_fast_finish()
>        error: we previously assumed 'key' could be null (see line 3209)
>
> net/mac80211/tx.c
>   3208        
>   3209                if (key)
>                     ^^^
> Check.
>
>   3210                        info->control.hw_key = &key->conf;
>   3211        
>   3212                ieee80211_tx_stats(skb->dev, skb->len);
>   3213        
>   3214                if (hdr->frame_control & 
> cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) {
>   3215                        tid = skb->priority & 
> IEEE80211_QOS_CTL_TAG1D_MASK;
>   3216                        *ieee80211_get_qos_ctl(hdr) = tid;
>   3217                        hdr->seq_ctrl = ieee80211_tx_next_seq(sta, tid);
>   3218                } else {
>   3219                        info->flags |= IEEE80211_TX_CTL_ASSIGN_SEQ;
>   3220                        hdr->seq_ctrl = 
> cpu_to_le16(sdata->sequence_number);
>   3221                        sdata->sequence_number += 0x10;
>   3222                }
>   3223        
>   3224                if (skb_shinfo(skb)->gso_size)
>   3225                        sta->tx_stats.msdu[tid] +=
>   3226                                DIV_ROUND_UP(skb->len, 
> skb_shinfo(skb)->gso_size);
>   3227                else
>   3228                        sta->tx_stats.msdu[tid]++;
>   3229        
>   3230                info->hw_queue = 
> sdata->vif.hw_queue[skb_get_queue_mapping(skb)];
>   3231        
>   3232                /* statistics normally done by ieee80211_tx_h_stats 
> (but that
>   3233                 * has to consider fragmentation, so is more complex)
>   3234                 */
>   3235                sta->tx_stats.bytes[skb_get_queue_mapping(skb)] += 
> skb->len;
>   3236                sta->tx_stats.packets[skb_get_queue_mapping(skb)]++;
>   3237        
>   3238                if (pn_offs) {
>                     ^^^^^^^
> Maybe when pn_offs is non-zero that implies key is non-NULL?

Yes, it does. fast_tx->pn_offs is set in ieee80211_check_fast_xmit()
which only sets it if fast_tx->key is set. The other call to
ieee80211_xmit_fast_finish() is in ieee80211_tx_dequeue() which also
only sets pn_offs if the key is set.

-Toke

Reply via email to