Re: [PATCH][net-next] gianfar: Fix alloc_skb_resources on -ENOMEM cleanup path

[Claudiu Manoil <claudiu.manoil@freescale.com>]

On 11/8/2012 4:21 PM, Paul Gortmaker wrote:
> On 12-11-08 08:40 AM, Claudiu Manoil wrote:
>> Should gfar_init_bds() return with -ENOMEM inside gfar_alloc_skb_resources(),
>> free_skb_resources() will be called twice in a row on the "cleanup" path,
>> leading to duplicate kfree() calls for rx_|tx_queue->rx_|tx_skbuff resulting
>> in segmentation fault.
>> This patch prevents the segmentation fault to happen in the future
>> (rx_|tx_sbkbuff set to NULL), and corrects the error path handling
>> for gfar_init_bds().
>
> Since gfar_init_bds is more like a slave routine to gfar_alloc_skb_resources,
> I think the dup free_skb_resources should remain in the parent, and be removed
> from gfar_init_bds.  Otherwise the gfar_alloc_skb_resources will appear
> confusing -- one will think it it allocates some resources, hits a failure
> and then returns without bothering to do any cleanup of the parts it
> did manage to allocate. (Then gfar_restore will have to call the free
> itself _if_ gfar_init_bds fails too.)
>
> Paul.

You're right. I'll send the v1 patch shortly.
Thanks.

Claudiu