On 11/28/2018 03:06 AM, Pan Bian wrote: > The buffer skb is freed via dev_kfree_skb in a loop. After freeing skb, > the value of packet_count is updated via packet_count++. If packet_count > happens to equal the upper bound budget, the loop will be broken and skb > may be assigned to rdata->state.skb. Resulting that rdata->state.skb may
Except that rdata->state.skb is only assigned if we have received something and it is (!last || context_next). If error is ever set, then we read descriptors as long as (!last || context_next). Also, packet->errors can only be set for the last packet which implies no following context descriptors. So rdata->state.skb can never be set with the "bad" skb value. The only way I can see this occurring is if a context descriptor is indicated (context_next) along with packet->errors. Are you actually seeing an error/crash where this is occurring? If so, then the fix should be to goto read_again if context_next is set because we want to remove the context descriptor in this case and not save the state. Thanks, Tom > point to a freed memory chunk. To fix this, the patch sets skb to NULL > after dev_kfree_skb(skb). > > Signed-off-by: Pan Bian <bianpan2...@163.com> > --- > V2: correct the commit log > --- > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > index 0cc911f..ac6b82d 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > @@ -2754,6 +2754,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, > int budget) > netif_err(pdata, rx_err, netdev, > "error in received packet\n"); > dev_kfree_skb(skb); > + skb = NULL; > goto next_packet; > } > > @@ -2806,6 +2807,7 @@ static int xgbe_rx_poll(struct xgbe_channel *channel, > int budget) > netif_err(pdata, rx_err, netdev, > "packet length exceeds configured MTU\n"); > dev_kfree_skb(skb); > + skb = NULL; > goto next_packet; > } > >
