2016-01-31 00:07:43

by Rafał Miłecki

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

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.

Signed-off-by: Rafał Miłecki <[email protected]>
---
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
--
1.8.4.5



2016-01-31 10:12:08

by Arend Van Spriel

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



On 31-01-16 10:56, Arend van Spriel wrote:
> 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 that patch on your device (not sure which one) ...

[let finish this sentence]... the patch never got applied to
wireless-drivers-next.

> 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.

So please Cc: [email protected] once reviewed by Hante.

Regards,
Arend

> Regards,
> Arend
>
> [1]
> http://thread.gmane.org/gmane.linux.kernel.wireless.general/141004/focus=141003
>
>> Signed-off-by: Rafał Miłecki <[email protected]>
>> ---
>> 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
>>

2016-01-31 11:43:34

by Rafał Miłecki

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

On 31 January 2016 at 10:56, Arend van Spriel <[email protected]> wrote:
> 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 wasn't aware Hante's patch contained changes from this patch. Anyway
the main difference is that my patch doesn't touch
BRCMF_FLOWRING_HASHSIZE.

So my patch:
1) Fixes possible overflows in flowrings

Hante's patch:
1) Fixes possible overflows in flowrings
2) Bumps BRCMF_FLOWRING_HASHSIZE

It was bumping BRCMF_FLOWRING_HASHSIZE that caused problems on my
BCM43602 device back then. Please note BCM43602 wasn't affected by
flowings overflows because it wasn't using more than 255 of them:
brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 132

The story is different with my BCM4366. I didn't try it with bumping
BRCMF_FLOWRING_HASHSIZE but it suffers from overflows in flowrings as
it seems to be independent issue. It's crucial that BCM4366 uses more
than 255 flowrings:
brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 264


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

I just realized BCM4366 support went into 4.4 not 4.5, so Cc-ing
stable for 4.4+ is probably a good idea.

2016-01-31 09:56:23

by Arend Van Spriel

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

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 <[email protected]>
> ---
> 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
>

2016-02-01 09:43:46

by Arend van Spriel

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

On 02/01/2016 09:46 AM, Hante Meuleman wrote:
> Hi,
>
> Didn’t know that that patch got reverted. Our internal repository still has that patch and we have had no problems whatsoever for the last 7 (june 26 this got submitted internally) months on any pcie target (also used it on r8000 openwrt). Rafal's patch is overdone. If you don’t up the hashsize space then there is really no use to switch to u16. You can simply limit the nr of flowrings to 255 in brcmf_proto_msgbuf_attach or apply the patch I originally submitted.

Ok. While no issues were seen I think we can not ignore the reported
problem.

Rafal,

Can you try Hante's patch on your current branch, ie. incorporate hash
size change and see if this shows any issues on 43602 target. If so full
driver logging would be great so we can look at that.

Regards,
Arend

> Regards,
> Hante
>
> -----Original Message-----
> From: Rafał Miłecki [mailto:[email protected]]
> Sent: Sunday, January 31, 2016 12:44 PM
> To: Arend van Spriel
> Cc: Kalle Valo; [email protected]; Brett Rudley; Arend Van Spriel; Franky (Zhenhui) Lin; Hante Meuleman; brcm80211-dev-list
> Subject: Re: [PATCH FIX?] brcmfmac: fix possible overflows in flowrings code by bumping u8 to u16
>
> On 31 January 2016 at 10:56, Arend van Spriel <[email protected]> wrote:
>> 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 wasn't aware Hante's patch contained changes from this patch. Anyway
> the main difference is that my patch doesn't touch
> BRCMF_FLOWRING_HASHSIZE.
>
> So my patch:
> 1) Fixes possible overflows in flowrings
>
> Hante's patch:
> 1) Fixes possible overflows in flowrings
> 2) Bumps BRCMF_FLOWRING_HASHSIZE
>
> It was bumping BRCMF_FLOWRING_HASHSIZE that caused problems on my
> BCM43602 device back then. Please note BCM43602 wasn't affected by
> flowings overflows because it wasn't using more than 255 of them:
> brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 132
>
> The story is different with my BCM4366. I didn't try it with bumping
> BRCMF_FLOWRING_HASHSIZE but it suffers from overflows in flowrings as
> it seems to be independent issue. It's crucial that BCM4366 uses more
> than 255 flowrings:
> brcmfmac: brcmf_pcie_init_ringbuffers Nr of flowrings is 264
>
>
>> I want Hante to review your patch, but indeed this would be 4.5 material
>> and probably stable.
>
> I just realized BCM4366 support went into 4.4 not 4.5, so Cc-ing
> stable for 4.4+ is probably a good idea.
>


2016-02-01 08:46:07

by Hante Meuleman

[permalink] [raw]
Subject: RE: [PATCH FIX?] brcmfmac: fix possible overflows in flowrings code by bumping u8 to u16

SGksDQoNCkRpZG7igJl0IGtub3cgdGhhdCB0aGF0IHBhdGNoIGdvdCByZXZlcnRlZC4gT3VyIGlu
dGVybmFsIHJlcG9zaXRvcnkgc3RpbGwgaGFzIHRoYXQgcGF0Y2ggYW5kIHdlIGhhdmUgaGFkIG5v
IHByb2JsZW1zIHdoYXRzb2V2ZXIgZm9yIHRoZSBsYXN0IDcgKGp1bmUgMjYgdGhpcyBnb3Qgc3Vi
bWl0dGVkIGludGVybmFsbHkpIG1vbnRocyBvbiBhbnkgcGNpZSB0YXJnZXQgKGFsc28gdXNlZCBp
dCBvbiByODAwMCBvcGVud3J0KS4gUmFmYWwncyBwYXRjaCBpcyBvdmVyZG9uZS4gSWYgeW91IGRv
buKAmXQgdXAgdGhlIGhhc2hzaXplIHNwYWNlIHRoZW4gdGhlcmUgaXMgcmVhbGx5IG5vIHVzZSB0
byBzd2l0Y2ggdG8gdTE2LiBZb3UgY2FuIHNpbXBseSBsaW1pdCB0aGUgbnIgb2YgZmxvd3Jpbmdz
IHRvIDI1NSBpbiBicmNtZl9wcm90b19tc2didWZfYXR0YWNoIG9yIGFwcGx5IHRoZSBwYXRjaCBJ
IG9yaWdpbmFsbHkgc3VibWl0dGVkLg0KDQpSZWdhcmRzLA0KSGFudGUNCg0KLS0tLS1PcmlnaW5h
bCBNZXNzYWdlLS0tLS0NCkZyb206IFJhZmHFgiBNacWCZWNraSBbbWFpbHRvOnphamVjNUBnbWFp
bC5jb21dIA0KU2VudDogU3VuZGF5LCBKYW51YXJ5IDMxLCAyMDE2IDEyOjQ0IFBNDQpUbzogQXJl
bmQgdmFuIFNwcmllbA0KQ2M6IEthbGxlIFZhbG87IGxpbnV4LXdpcmVsZXNzQHZnZXIua2VybmVs
Lm9yZzsgQnJldHQgUnVkbGV5OyBBcmVuZCBWYW4gU3ByaWVsOyBGcmFua3kgKFpoZW5odWkpIExp
bjsgSGFudGUgTWV1bGVtYW47IGJyY204MDIxMS1kZXYtbGlzdA0KU3ViamVjdDogUmU6IFtQQVRD
SCBGSVg/XSBicmNtZm1hYzogZml4IHBvc3NpYmxlIG92ZXJmbG93cyBpbiBmbG93cmluZ3MgY29k
ZSBieSBidW1waW5nIHU4IHRvIHUxNg0KDQpPbiAzMSBKYW51YXJ5IDIwMTYgYXQgMTA6NTYsIEFy
ZW5kIHZhbiBTcHJpZWwgPGFzcHJpZWxAZ21haWwuY29tPiB3cm90ZToNCj4gT24gMzEtMDEtMTYg
MDE6MDcsIFJhZmHFgiBNacWCZWNraSB3cm90ZToNCj4+IFNvbWUgZGV2aWNlcyBtYXkgdXNlIG1v
cmUgdGhhbiAyNTUgZmxvd2luZ3MsIGJlbG93IGlzIGxvZyBmcm9tIEJDTTQzNjY6DQo+PiBbICAx
OTQuNjA2MjQ1XSBicmNtZm1hYzogYnJjbWZfcGNpZV9pbml0X3JpbmdidWZmZXJzIE5yIG9mIGZs
b3dyaW5ncyBpcyAyNjQNCj4+DQo+PiBBdCB2YXJpb3VzIHBsYWNlcyB3ZSB3ZXJlIHVzaW5nIHU4
IHdoaWNoIGNvdWxkIGxlYWQgdG8gc3RvcmluZyB3cm9uZw0KPj4gbnVtYmVyIG9yIGluZmluaXRl
IGxvb3BzIHdoZW4gaW5kZXhpbmcgaW5jb3JyZWN0bHkuIEluaXRpYWxseSB0aGlzDQo+PiBpc3N1
ZSB3YXMgc3BvdHRlZCBhcyBpbmZpbml0ZSBsb29wIGluIGJyY21mX2Zsb3dyaW5nX2RldGFjaC4N
Cj4NCj4gVGhlcmUgaGFzIGFscmVhZHkgYmVlbiBhIHBhdGNoIHN1Ym1pdHRlZCBmb3IgdGhpcyBb
MV0uIEhvd2V2ZXIsIGJlY2F1c2UNCj4geW91IHJlcG9ydGVkIGlzc3VlcyB3aXRoIGl0IG9uIHlv
dXIgZGV2aWNlIChub3Qgc3VyZSB3aGljaCBvbmUpLiBEaWQgeW91DQo+IHRlc3QgdGhpcyBwYXRj
aCBvbiB0aGF0IHBhcnRpY3VsYXIgZGV2aWNlLg0KDQpJIHdhc24ndCBhd2FyZSBIYW50ZSdzIHBh
dGNoIGNvbnRhaW5lZCBjaGFuZ2VzIGZyb20gdGhpcyBwYXRjaC4gQW55d2F5DQp0aGUgbWFpbiBk
aWZmZXJlbmNlIGlzIHRoYXQgbXkgcGF0Y2ggZG9lc24ndCB0b3VjaA0KQlJDTUZfRkxPV1JJTkdf
SEFTSFNJWkUuDQoNClNvIG15IHBhdGNoOg0KMSkgRml4ZXMgcG9zc2libGUgb3ZlcmZsb3dzIGlu
IGZsb3dyaW5ncw0KDQpIYW50ZSdzIHBhdGNoOg0KMSkgRml4ZXMgcG9zc2libGUgb3ZlcmZsb3dz
IGluIGZsb3dyaW5ncw0KMikgQnVtcHMgQlJDTUZfRkxPV1JJTkdfSEFTSFNJWkUNCg0KSXQgd2Fz
IGJ1bXBpbmcgQlJDTUZfRkxPV1JJTkdfSEFTSFNJWkUgdGhhdCBjYXVzZWQgcHJvYmxlbXMgb24g
bXkNCkJDTTQzNjAyIGRldmljZSBiYWNrIHRoZW4uIFBsZWFzZSBub3RlIEJDTTQzNjAyIHdhc24n
dCBhZmZlY3RlZCBieQ0KZmxvd2luZ3Mgb3ZlcmZsb3dzIGJlY2F1c2UgaXQgd2Fzbid0IHVzaW5n
IG1vcmUgdGhhbiAyNTUgb2YgdGhlbToNCmJyY21mbWFjOiBicmNtZl9wY2llX2luaXRfcmluZ2J1
ZmZlcnMgTnIgb2YgZmxvd3JpbmdzIGlzIDEzMg0KDQpUaGUgc3RvcnkgaXMgZGlmZmVyZW50IHdp
dGggbXkgQkNNNDM2Ni4gSSBkaWRuJ3QgdHJ5IGl0IHdpdGggYnVtcGluZw0KQlJDTUZfRkxPV1JJ
TkdfSEFTSFNJWkUgYnV0IGl0IHN1ZmZlcnMgZnJvbSBvdmVyZmxvd3MgaW4gZmxvd3JpbmdzIGFz
DQppdCBzZWVtcyB0byBiZSBpbmRlcGVuZGVudCBpc3N1ZS4gSXQncyBjcnVjaWFsIHRoYXQgQkNN
NDM2NiB1c2VzIG1vcmUNCnRoYW4gMjU1IGZsb3dyaW5nczoNCmJyY21mbWFjOiBicmNtZl9wY2ll
X2luaXRfcmluZ2J1ZmZlcnMgTnIgb2YgZmxvd3JpbmdzIGlzIDI2NA0KDQoNCj4gSSB3YW50IEhh
bnRlIHRvIHJldmlldyB5b3VyIHBhdGNoLCBidXQgaW5kZWVkIHRoaXMgd291bGQgYmUgNC41IG1h
dGVyaWFsDQo+IGFuZCBwcm9iYWJseSBzdGFibGUuDQoNCkkganVzdCByZWFsaXplZCBCQ000MzY2
IHN1cHBvcnQgd2VudCBpbnRvIDQuNCBub3QgNC41LCBzbyBDYy1pbmcNCnN0YWJsZSBmb3IgNC40
KyBpcyBwcm9iYWJseSBhIGdvb2QgaWRlYS4NCg0K

2016-09-03 14:15:40

by Kalle Valo

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

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.
>
> Signed-off-by: Rafa? Mi?ecki <[email protected]>

There has been no activity on this patch so I'll drop this. Please resend if
this is still needed.

--
Sent by pwcli
https://patchwork.kernel.org/patch/8172531/