Search Linux Wireless

Re: [PATCH FIX?] brcmfmac: fix possible overflows in flowrings code by bumping u8 to u16

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 31-01-16 01:07, Rafał Miłecki wrote:
> Some devices may use more than 255 flowings, below is log from BCM4366:
> [  194.606245] brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 264
> 
> At various places we were using u8 which could lead to storing wrong
> number or infinite loops when indexing incorrectly. Initially this
> issue was spotted as infinite loop in brcmf_flowring_detach.

There has already been a patch submitted for this [1]. However, because
you reported issues with it on your device (not sure which one). Did you
test this patch on that particular device.

I want Hante to review your patch, but indeed this would be 4.5 material
and probably stable.

Regards,
Arend

[1]
http://thread.gmane.org/gmane.linux.kernel.wireless.general/141004/focus=141003

> Signed-off-by: Rafał Miłecki <zajec5@xxxxxxxxx>
> ---
> I guess it's a good candidate for a fix (4.5 material). Any objections?
> ---
>  .../broadcom/brcm80211/brcmfmac/flowring.c         | 24 +++++++++++-----------
>  .../broadcom/brcm80211/brcmfmac/flowring.h         | 18 ++++++++--------
>  .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c  |  4 ++--
>  .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.h  |  2 +-
>  4 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> index 2ca783f..3d2373b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> @@ -169,7 +169,7 @@ u32 brcmf_flowring_create(struct brcmf_flowring *flow, u8 da[ETH_ALEN],
>  }
>  
>  
> -u8 brcmf_flowring_tid(struct brcmf_flowring *flow, u8 flowid)
> +u8 brcmf_flowring_tid(struct brcmf_flowring *flow, u16 flowid)
>  {
>  	struct brcmf_flowring_ring *ring;
>  
> @@ -179,7 +179,7 @@ u8 brcmf_flowring_tid(struct brcmf_flowring *flow, u8 flowid)
>  }
>  
>  
> -static void brcmf_flowring_block(struct brcmf_flowring *flow, u8 flowid,
> +static void brcmf_flowring_block(struct brcmf_flowring *flow, u16 flowid,
>  				 bool blocked)
>  {
>  	struct brcmf_flowring_ring *ring;
> @@ -228,7 +228,7 @@ static void brcmf_flowring_block(struct brcmf_flowring *flow, u8 flowid,
>  }
>  
>  
> -void brcmf_flowring_delete(struct brcmf_flowring *flow, u8 flowid)
> +void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
>  {
>  	struct brcmf_flowring_ring *ring;
>  	u8 hash_idx;
> @@ -253,7 +253,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u8 flowid)
>  }
>  
>  
> -u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
> +u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u16 flowid,
>  			   struct sk_buff *skb)
>  {
>  	struct brcmf_flowring_ring *ring;
> @@ -279,7 +279,7 @@ u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
>  }
>  
>  
> -struct sk_buff *brcmf_flowring_dequeue(struct brcmf_flowring *flow, u8 flowid)
> +struct sk_buff *brcmf_flowring_dequeue(struct brcmf_flowring *flow, u16 flowid)
>  {
>  	struct brcmf_flowring_ring *ring;
>  	struct sk_buff *skb;
> @@ -300,7 +300,7 @@ struct sk_buff *brcmf_flowring_dequeue(struct brcmf_flowring *flow, u8 flowid)
>  }
>  
>  
> -void brcmf_flowring_reinsert(struct brcmf_flowring *flow, u8 flowid,
> +void brcmf_flowring_reinsert(struct brcmf_flowring *flow, u16 flowid,
>  			     struct sk_buff *skb)
>  {
>  	struct brcmf_flowring_ring *ring;
> @@ -311,7 +311,7 @@ void brcmf_flowring_reinsert(struct brcmf_flowring *flow, u8 flowid,
>  }
>  
>  
> -u32 brcmf_flowring_qlen(struct brcmf_flowring *flow, u8 flowid)
> +u32 brcmf_flowring_qlen(struct brcmf_flowring *flow, u16 flowid)
>  {
>  	struct brcmf_flowring_ring *ring;
>  
> @@ -326,7 +326,7 @@ u32 brcmf_flowring_qlen(struct brcmf_flowring *flow, u8 flowid)
>  }
>  
>  
> -void brcmf_flowring_open(struct brcmf_flowring *flow, u8 flowid)
> +void brcmf_flowring_open(struct brcmf_flowring *flow, u16 flowid)
>  {
>  	struct brcmf_flowring_ring *ring;
>  
> @@ -340,7 +340,7 @@ void brcmf_flowring_open(struct brcmf_flowring *flow, u8 flowid)
>  }
>  
>  
> -u8 brcmf_flowring_ifidx_get(struct brcmf_flowring *flow, u8 flowid)
> +u8 brcmf_flowring_ifidx_get(struct brcmf_flowring *flow, u16 flowid)
>  {
>  	struct brcmf_flowring_ring *ring;
>  	u8 hash_idx;
> @@ -384,7 +384,7 @@ void brcmf_flowring_detach(struct brcmf_flowring *flow)
>  	struct brcmf_pub *drvr = bus_if->drvr;
>  	struct brcmf_flowring_tdls_entry *search;
>  	struct brcmf_flowring_tdls_entry *remove;
> -	u8 flowid;
> +	u16 flowid;
>  
>  	for (flowid = 0; flowid < flow->nrofrings; flowid++) {
>  		if (flow->rings[flowid])
> @@ -408,7 +408,7 @@ void brcmf_flowring_configure_addr_mode(struct brcmf_flowring *flow, int ifidx,
>  	struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
>  	struct brcmf_pub *drvr = bus_if->drvr;
>  	u32 i;
> -	u8 flowid;
> +	u16 flowid;
>  
>  	if (flow->addr_mode[ifidx] != addr_mode) {
>  		for (i = 0; i < ARRAY_SIZE(flow->hash); i++) {
> @@ -434,7 +434,7 @@ void brcmf_flowring_delete_peer(struct brcmf_flowring *flow, int ifidx,
>  	struct brcmf_flowring_tdls_entry *prev;
>  	struct brcmf_flowring_tdls_entry *search;
>  	u32 i;
> -	u8 flowid;
> +	u16 flowid;
>  	bool sta;
>  
>  	sta = (flow->addr_mode[ifidx] == ADDR_INDIRECT);
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.h
> index 95fd1c9..c59f684 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.h
> @@ -24,7 +24,7 @@ struct brcmf_flowring_hash {
>  	u8 mac[ETH_ALEN];
>  	u8 fifo;
>  	u8 ifidx;
> -	u8 flowid;
> +	u16 flowid;
>  };
>  
>  enum ring_status {
> @@ -61,16 +61,16 @@ u32 brcmf_flowring_lookup(struct brcmf_flowring *flow, u8 da[ETH_ALEN],
>  			  u8 prio, u8 ifidx);
>  u32 brcmf_flowring_create(struct brcmf_flowring *flow, u8 da[ETH_ALEN],
>  			  u8 prio, u8 ifidx);
> -void brcmf_flowring_delete(struct brcmf_flowring *flow, u8 flowid);
> -void brcmf_flowring_open(struct brcmf_flowring *flow, u8 flowid);
> -u8 brcmf_flowring_tid(struct brcmf_flowring *flow, u8 flowid);
> -u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u8 flowid,
> +void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid);
> +void brcmf_flowring_open(struct brcmf_flowring *flow, u16 flowid);
> +u8 brcmf_flowring_tid(struct brcmf_flowring *flow, u16 flowid);
> +u32 brcmf_flowring_enqueue(struct brcmf_flowring *flow, u16 flowid,
>  			   struct sk_buff *skb);
> -struct sk_buff *brcmf_flowring_dequeue(struct brcmf_flowring *flow, u8 flowid);
> -void brcmf_flowring_reinsert(struct brcmf_flowring *flow, u8 flowid,
> +struct sk_buff *brcmf_flowring_dequeue(struct brcmf_flowring *flow, u16 flowid);
> +void brcmf_flowring_reinsert(struct brcmf_flowring *flow, u16 flowid,
>  			     struct sk_buff *skb);
> -u32 brcmf_flowring_qlen(struct brcmf_flowring *flow, u8 flowid);
> -u8 brcmf_flowring_ifidx_get(struct brcmf_flowring *flow, u8 flowid);
> +u32 brcmf_flowring_qlen(struct brcmf_flowring *flow, u16 flowid);
> +u8 brcmf_flowring_ifidx_get(struct brcmf_flowring *flow, u16 flowid);
>  struct brcmf_flowring *brcmf_flowring_attach(struct device *dev, u16 nrofrings);
>  void brcmf_flowring_detach(struct brcmf_flowring *flow);
>  void brcmf_flowring_configure_addr_mode(struct brcmf_flowring *flow, int ifidx,
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> index c2bdb91..0b9c2dd 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
> @@ -677,7 +677,7 @@ static u32 brcmf_msgbuf_flowring_create(struct brcmf_msgbuf *msgbuf, int ifidx,
>  }
>  
>  
> -static void brcmf_msgbuf_txflow(struct brcmf_msgbuf *msgbuf, u8 flowid)
> +static void brcmf_msgbuf_txflow(struct brcmf_msgbuf *msgbuf, u16 flowid)
>  {
>  	struct brcmf_flowring *flow = msgbuf->flow;
>  	struct brcmf_commonring *commonring;
> @@ -1310,7 +1310,7 @@ int brcmf_proto_msgbuf_rx_trigger(struct device *dev)
>  }
>  
>  
> -void brcmf_msgbuf_delete_flowring(struct brcmf_pub *drvr, u8 flowid)
> +void brcmf_msgbuf_delete_flowring(struct brcmf_pub *drvr, u16 flowid)
>  {
>  	struct brcmf_msgbuf *msgbuf = (struct brcmf_msgbuf *)drvr->proto->pd;
>  	struct msgbuf_tx_flowring_delete_req *delete;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.h
> index 3d513e4..ee6906a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.h
> @@ -33,7 +33,7 @@
>  
>  
>  int brcmf_proto_msgbuf_rx_trigger(struct device *dev);
> -void brcmf_msgbuf_delete_flowring(struct brcmf_pub *drvr, u8 flowid);
> +void brcmf_msgbuf_delete_flowring(struct brcmf_pub *drvr, u16 flowid);
>  int brcmf_proto_msgbuf_attach(struct brcmf_pub *drvr);
>  void brcmf_proto_msgbuf_detach(struct brcmf_pub *drvr);
>  #else
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux