netdev - Re: [PATCH][net-next] gianfar: Fix alloc_skb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 8 Nov 2012 17:37:03 +0200
From:	Claudiu Manoil <claudiu.manoil@...escale.com>
To:	Paul Gortmaker <paul.gortmaker@...driver.com>
CC:	<netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH][net-next] gianfar: Fix alloc_skb_resources on -ENOMEM
 cleanup path

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


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html