Fix possible skb_over_panic event in Freescale's "gianfar" driver.
The skb_over_panic occurs due to calling skb_put() within
gfar_clean_rx_ring(). This happens if (and only if) shortly prior to the
event and a few lined above the skb_put(), an skb was queued back to the
priv->rx_recycle queue due to RXBD_LAST or RXBD_ERR status.
The skb is queued without properly re-setting its state.
The patch properly reset the skb state.
I have tested this patch on MPC8548 based product and asserted it avoided the skb_over_panic event.
Signed-off-by: Eran Liberty <[email protected]>
---
From: Eran Liberty <[email protected]>
Date: Thu, 24 Jun 2010 12:53:23 +0300
> Fix possible skb_over_panic event in Freescale's "gianfar" driver.
>
> The skb_over_panic occurs due to calling skb_put() within
> gfar_clean_rx_ring(). This happens if (and only if) shortly prior to
> the event and a few lined above the skb_put(), an skb was queued back
> to the priv->rx_recycle queue due to RXBD_LAST or RXBD_ERR status.
> The skb is queued without properly re-setting its state.
>
> The patch properly reset the skb state.
>
> I have tested this patch on MPC8548 based product and asserted it
> avoided the skb_over_panic event.
>
> Signed-off-by: Eran Liberty <[email protected]>
Eran, this seems to be fixed already. The code in the current
tree now reads:
/*
* We need to un-reserve() the skb to what it
* was before gfar_new_skb() re-aligned
* it to an RXBUF_ALIGNMENT boundary
* before we put the skb back on the
* recycle list.
*/
skb_reserve(skb, -GFAR_CB(skb)->alignamount);
__skb_queue_head(&priv->rx_recycle, skb);
David Miller wrote:
> From: Eran Liberty <[email protected]>
> Date: Thu, 24 Jun 2010 12:53:23 +0300
>
>
>> Fix possible skb_over_panic event in Freescale's "gianfar" driver.
>>
>> The skb_over_panic occurs due to calling skb_put() within
>> gfar_clean_rx_ring(). This happens if (and only if) shortly prior to
>> the event and a few lined above the skb_put(), an skb was queued back
>> to the priv->rx_recycle queue due to RXBD_LAST or RXBD_ERR status.
>> The skb is queued without properly re-setting its state.
>>
>> The patch properly reset the skb state.
>>
>> I have tested this patch on MPC8548 based product and asserted it
>> avoided the skb_over_panic event.
>>
>> Signed-off-by: Eran Liberty <[email protected]>
>>
>
> Eran, this seems to be fixed already. The code in the current
> tree now reads:
>
> /*
> * We need to un-reserve() the skb to what it
> * was before gfar_new_skb() re-aligned
> * it to an RXBUF_ALIGNMENT boundary
> * before we put the skb back on the
> * recycle list.
> */
> skb_reserve(skb, -GFAR_CB(skb)->alignamount);
> __skb_queue_head(&priv->rx_recycle, skb);
>
>
>
This code has proved to be insufficient and produce skb_over_panic. The
proposed patch fix this.
-- Liberty
From: Eran Liberty <[email protected]>
Date: Mon, 28 Jun 2010 10:57:08 +0300
> This code has proved to be insufficient and produce
> skb_over_panic. The proposed patch fix this.
Then you have to post a patch relative to the current code, rather than
against the code as it was several releases ago.
Your patch didn't apply, so I can't use it.
David Miller wrote:
> From: Eran Liberty <[email protected]>
> Date: Mon, 28 Jun 2010 10:57:08 +0300
>
>
>> This code has proved to be insufficient and produce
>> skb_over_panic. The proposed patch fix this.
>>
>
> Then you have to post a patch relative to the current code, rather than
> against the code as it was several releases ago.
>
> Your patch didn't apply, so I can't use it.
>
>
Upon cleaning up my patch for the latest kernel I realized I do not
like: the previous partial fix, the fix in ucc_geth.c, the fix in the
current latest kernel, and my own previously proposed patch. They all
tried to undo the alignment skb_reserve done in gfar_new_skb() before
queuing the skb into the rw_recycle, because upon getting a new one in
gfar_new_skb() if the skb is from the rx_recycle pool rather then newly
allocated it is reserved twice.
Instead of trying to undo the skb_reserve this proposed patch will make
sure the alignment skb_reserve is done once, upon allocating the skb and
not when taken out of the rx_recycle pool. Eliminating the need to undo
anything before queue skb back to the pool.
This patch will apply cleanly against the 2.6.32.15. Another patch will
be submitted separately for the current Linus tree.
-- Liberty
Signed-off-by: Eran Liberty <[email protected]>
This patch relates to "[PATCH] gainfar.c : skb_over_panic
(kernel-2.6.32.15)"
While in 2.6.32.15 it actually fixed a bug here it merely cleans up the
previous attempts to fix the bug with a more coherent code.
Currently before queuing skb into the rx_recycle it is
"un-skb_reserve"-ed so when taken out in gfar_new_skb() it wont be
reserved twice.
This patch makes sure the alignment skb_reserve is done once, upon
allocating the skb and
not when taken out of the rx_recycle pool. Eliminating the need to undo
anything before queue skb back to the pool.
NOTE: This patch will compile and is fairly straight forward but I do
not have environment to test it as I did with the 2.6.32.15 fix.
-- Liberty
Signed-off-by: Eran Liberty <[email protected]>