netdev - Re: [PATCH 1/1] ehea: Allocate stats buffer with GFP

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <29190.1282153781@death>
Date:	Wed, 18 Aug 2010 10:49:41 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
cc:	David Miller <davem@...emloft.net>, brking@...ux.vnet.ibm.com,
	ossthema@...ibm.com, osstklei@...ibm.com, raisch@...ibm.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] ehea: Allocate stats buffer with GFP_KERNEL

Eric Dumazet <eric.dumazet@...il.com> wrote:

>Le jeudi 01 juillet 2010 à 22:48 -0700, David Miller a écrit :
>> From: Brian King <brking@...ux.vnet.ibm.com>
>> Date: Wed, 30 Jun 2010 16:59:12 -0500
>> 
>> > 
>> > Since ehea_get_stats calls ehea_h_query_ehea_port, which
>> > can sleep, we can also sleep when allocating a page in
>> > this function. This fixes some memory allocation failure
>> > warnings seen under low memory conditions.
>> > 
>> > Signed-off-by: Brian King <brking@...ux.vnet.ibm.com>
>> 
>> Applied to net-next-2.6
>> --
>
>I believe there is a problem with this patch and/or bonding.
>
>If we say ndo_get_stats() methods are allowed to sleep, then 
>bond_get_stats() should be updated, because it currently calls
>dev_get_stats() from a read_lock_bh(&bond->lock); section.
>
>Are we allowed to sleep inside a read_lock_bh() ?

	Nope.

	And bonding's not the only call site that holds a lock over the
call to ndo_get_stats / dev_get_stats; net/core/net-sysfs.c:netstat_show
does it as well.

	I presume that bonding and netstat_show are holding a lock to
keep a list of interfaces from changing, since there's no other locking
that's guaranteed to be held for a call to dev_get_stats.

	In any event, ehea is doing an hcall to the hypervisor, which
may return "long busy," after which ehea sleeps for however long the
hypervisor told it to wait before trying again.

	So, the real question is whether the ndo_get_stats* functions
are permitted to sleep.  If they are, then bonding and netstat_show both
need to change.  If not, then ehea needs to change.  Ehea is probably
not alone in this; I poked around a bit, and it looks like mlx4 may also
sleep in ndo_get_stats.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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