netdev - Re: [PATCH 2/7] netpoll: make __netpoll

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]

Date:	Mon, 30 Jul 2012 09:42:01 +0800
From:	Cong Wang <amwang@...hat.com>
To:	Neil Horman <nhorman@...driver.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Jay Vosburgh <fubar@...ibm.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Patrick McHardy <kaber@...sh.net>,
	Stephen Hemminger <shemminger@...tta.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Eric Dumazet <edumazet@...gle.com>,
	Cong Wang <xiyou.wangcong@...il.com>,
	Joe Perches <joe@...ches.com>, linux-kernel@...r.kernel.org,
	bridge@...ts.linux-foundation.org
Subject: Re: [PATCH 2/7] netpoll: make __netpoll_cleanup non-block

On Fri, 2012-07-27 at 14:40 -0400, Neil Horman wrote:
> Here, and above I see you using an rcu_head to defer cleanup, until after all
> pointer uses are dropped, but I don't see any modification of code points that
> dereference any struct netpoll pointers to include
> rcu_read_lock()/rcu_read_unlock().  Without those using rcu to defer cleanup is
> pointless, as the rcu code won't know when its safe to run.  You're no better
> off that you would be just calling __netpoll_cleanup directly.

Hi, Neil,

Actually they are protected by rcu_read_lock_bh() which is implied by
local_irq_disable(), see:

commit f0f9deae9e7c421fa0c1c627beb8e174325e1ba7
Author: Herbert Xu <herbert@...dor.apana.org.au>
Date:   Fri Sep 17 16:55:03 2010 -0700

    netpoll: Disable IRQ around RCU dereference in netpoll_rx
    
    We cannot use rcu_dereference_bh safely in netpoll_rx as we may
    be called with IRQs disabled.  We could however simply disable
    IRQs as that too causes BH to be disabled and is safe in either
    case.
    
    Thanks to John Linville for discovering this bug and providing
    a patch.
    
    Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
    Signed-off-by: David S. Miller <davem@...emloft.net>


Thanks.


--
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