[bug report] wlcore/wl18xx: Add functionality to accept TX rate per link

[bug report] wlcore/wl18xx: Add functionality to accept TX rate per link

[Dan Carpenter]

Hello Maxim Altshul,

The patch 88f07e70d1be: "wlcore/wl18xx: Add functionality to accept
TX rate per link" from Jul 11, 2016, leads to the following static
checker warning:

	drivers/net/wireless/ti/wl18xx/tx.c:35 wl18xx_get_last_tx_rate()
	error: buffer overflow 'wl->links' 16 <= 255

drivers/net/wireless/ti/wl18xx/tx.c
    31  static
    32  void wl18xx_get_last_tx_rate(struct wl1271 *wl, struct ieee80211_vif *vif,
    33                               u8 band, struct ieee80211_tx_rate *rate, u8 hlid)
    34  {
    35          u8 fw_rate = wl->links[hlid].fw_rate_idx;
                                       ^^^^
The complaint here is that we take "hlid" from skb->data without
verifying that it's correct.  But this is the tx path and not the rx
path so it might be a false positive if we verify that it's correct
earlier.  I don't know the code well enough to say.

    36  
    37          if (fw_rate > CONF_HW_RATE_INDEX_MAX) {
    38                  wl1271_error("last Tx rate invalid: %d", fw_rate);
    39                  rate->idx = 0;
    40                  rate->flags = 0;
    41                  return;
    42          }
    43  
    44          if (fw_rate <= CONF_HW_RATE_INDEX_54MBPS) {
    45                  rate->idx = fw_rate;
    46                  if (band = NL80211_BAND_5GHZ)
    47                          rate->idx -= CONF_HW_RATE_INDEX_6MBPS;
    48                  rate->flags = 0;

regards,
dan carpenter

Re: [bug report] wlcore/wl18xx: Add functionality to accept TX rate per link

[Altshul, Maxim]

SGksIGlmIEkgYW0gbm90IG1pc3Rha2VuIHRoZW4gdGhlIHNrYiBnb2VzIHRocm91Z2ggdGhlIGZv
bGxvd2luZyBwcm9jZXNzOiAxLiBJdCBpcyBjcmVhdGVkIGJ5IHRoZSBkcml2ZXIgYW5kIGEgaGxp
ZCBpcyBhc3NpZ25lZC4gCjIuIEl0IGlzIHBhc3NlZCB0byB0aGUgRlcgZm9yIHRyYW5zbWlzc2lv
bi4gCjMuIFRoZSBobGlkIGlzIGNoZWNrZWQgYnkgdGhlIEZXIGJlZm9yZSBhbnl0aGluZyBoYXBw
ZW5zLiAKNC4gSXQgaXMgcmVsZWFzZWQgYnkgdGhlIEZXIGFmdGVyIHRyYW5zbWlzc2lvbi4gCjUu
IFRoZSBobGlkIGlzIHRha2VuIHRvIGdldCB0aGUgcmF0ZSBmcm9tIGxpbmtzIGFycmF5LiAKCklu
IG15IG9waW5pb24sIG5vdGhpbmcgYmFkIHNob3VsZCBoYXBwZW4sIGFzIHRoZSBobGlkIGlzIGNy
ZWF0ZWQgYnkgdGhlIGRyaXZlciBpdHNlbGYgYW5kIGNoZWNrZWQgYnkgdGhlIEZXIGZvciBjb3Jy
ZWN0bmVzcy4gSSBjb3VsZCBhZGQgYW5vdGhlciBjaGVjayB0byB2ZXJpZnkgdGhhdCBobGlkIGRv
ZXMgbm90IGV4Y2VlZCB0aGUgc2l6ZSBvZiBsaW5rcyBhcnJheS4gCgpJIHdpbGwgdGFrZSBhIGxv
b2sgYXQgdGhlIGNvZGUgYXMgc29vbiBhcyBwb3NzaWJsZS4gCgpSZWdhcmRzLCAKTWF4

Re: [bug report] wlcore/wl18xx: Add functionality to accept TX rate per link

[Dan Carpenter]

On Tue, Oct 11, 2016 at 07:58:52AM +0000, Altshul, Maxim wrote:
> Hi, if I am not mistaken then the skb goes through the following process: 1. It is created by the driver and a hlid is assigned. 
> 2. It is passed to the FW for transmission. 
> 3. The hlid is checked by the FW before anything happens. 
> 4. It is released by the FW after transmission. 
> 5. The hlid is taken to get the rate from links array. 
> 

You're probably right, but it would be easier to follow along if there
were function names here.

regards,
dan carpenter