Re: [PATCH 10/10] rsi: drop RX broadcast/multicast packets with invalid PN

[Kalle Valo <kvalo@codeaurora.org>]

Amitkumar Karwar <amitkarwar@gmail.com> writes:

> From: Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>
>
> This patch adds a check to drop received broadcast/multicast frames if
> PN is invalid (i.e. not greater than last PN). bc_mc_pn
> variable added for each interface
>
> Signed-off-by: Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>
> Signed-off-by: Amitkumar Karwar <amit.karwar@redpinesignals.com>

[...]

> +static int rsi_validate_pn(struct rsi_hw *adapter, struct ieee80211_hdr *hdr)
> +{
> +	struct ieee80211_vif *vif;
> +	struct ieee80211_bss_conf *bss;
> +	struct vif_priv *vif_info = NULL;
> +	u8 cur_pn[IEEE80211_CCMP_PN_LEN];
> +	u8 *last_pn;
> +	int i, hdrlen;
> +
> +	if (!is_broadcast_ether_addr(hdr->addr1) &&
> +	    !is_multicast_ether_addr(hdr->addr1))
> +		return 1;
> +
> +	hdrlen = ieee80211_hdrlen(hdr->frame_control);
> +	for (i = 0; i < adapter->sc_nvifs; i++) {
> +		vif = adapter->vifs[i];
> +
> +		if (!vif)
> +			continue;
> +		if (vif->type != NL80211_IFTYPE_STATION &&
> +		    vif->type != NL80211_IFTYPE_P2P_CLIENT)
> +			continue;
> +		bss = &vif->bss_conf;
> +		if (!bss->assoc)
> +			continue;
> +		if (!ether_addr_equal(bss->bssid, hdr->addr2))
> +			continue;
> +		vif_info = (struct vif_priv *)vif->drv_priv;
> +		if (!vif_info->key) {
> +			vif_info = NULL;
> +			continue;
> +		}
> +		if (!vif_info->rx_pn_valid) {
> +			vif_info = NULL;
> +			continue;
> +		}
> +	}
> +	if (!vif_info)
> +		return 1;

Why +1 here?

> +	last_pn = vif_info->rx_bcmc_pn;
> +	if (vif_info->key->cipher == WLAN_CIPHER_SUITE_CCMP) {
> +		struct dot11_ccmp_hdr *ccmp =
> +			(struct dot11_ccmp_hdr *)&((u8 *)hdr)[hdrlen];
> +
> +		cur_pn[0] = ccmp->pn0;
> +		cur_pn[1] = ccmp->pn1;
> +		cur_pn[2] = ccmp->pn2;
> +		cur_pn[3] = ccmp->pn3;
> +		cur_pn[4] = ccmp->pn4;
> +		cur_pn[5] = ccmp->pn5;
> +	} else {
> +		struct dot11_tkip_hdr *tkip =
> +			(struct dot11_tkip_hdr *)&((u8 *)hdr)[hdrlen];
> +
> +		cur_pn[0] = tkip->tsc0;
> +		cur_pn[1] = tkip->tsc1;
> +		cur_pn[2] = tkip->tsc2;
> +		cur_pn[3] = tkip->tsc3;
> +		cur_pn[4] = tkip->tsc4;
> +		cur_pn[5] = tkip->tsc5;
> +	}
> +	for (i = (IEEE80211_CCMP_PN_LEN - 1); i >= 0; i--)
> +		if (last_pn[i] ^ cur_pn[i])
> +			break;
> +	if (i < 0)
> +		return -1;

And why -1 here? Please use real error codes (-EINVAL etc).

> @@ -1341,14 +1488,14 @@ static void rsi_fill_rx_status(struct ieee80211_hw *hw,
>  		}
>  	}
>  	if (!bss)
> -		return;
> +		return -1;

Here as well.

-- 
Kalle Valo