netdev - Re: [PATCH net-next v2] rocker: move netevent neigh update to processes context

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: <20150604090749.GA18460@verge.net.au>
Date:	Thu, 4 Jun 2015 18:07:52 +0900
From:	Simon Horman <horms@...ge.net.au>
To:	David Miller <davem@...emloft.net>
Cc:	simon.horman@...ronome.com, sfeldma@...il.com,
	netdev@...r.kernel.org, jiri@...nulli.us,
	makita.toshiaki@....ntt.co.jp
Subject: Re: [PATCH net-next v2] rocker: move netevent neigh update to
 processes context

Hi Dave,

On Thu, Jun 04, 2015 at 01:34:09AM -0700, David Miller wrote:
> From: Simon Horman <simon.horman@...ronome.com>
> Date: Thu, 4 Jun 2015 17:20:48 +0900
> 
> > What I was seeing is as follows:
> > 
> > 1. rocker_port_ipv4_nh() is called via switchdev_port_obj_add()
> >    with trans = SWITCHDEV_TRANS_PREPARE
> > 
> > 2. rocker_port_ipv4_neigh() is called by rocker_neigh_update()
> >    with trans = SWITCHDEV_TRANS_NONE.
> > 
> >    The call chain goes up to arp_process() via neigh_update().
> > 
> > 3. rocker_port_ipv4_nh() is called via switchdev_port_obj_add()
> >    with trans = SWITCHDEV_TRANS_COMMIT
> > 
> > #1 and #2 are guarded by rtl across those calls but
> > #2 is not guarded by rtnl.
> > 
> > Inside both rocker_port_ipv4_nh() and rocker_port_ipv4_neigh()
> > neigh_tbl_lock lock is taken but it is not held across the
> > two calls to rocker_port_ipv4_nh within a single prepare->commit transaction.
> > 
> > I can double check that the above still occurs, but I'm not aware of any
> > recent changes that would cause it not to occur any more.
> 
> Ok, thanks for explaining.
> 
> I still don't want this to be moved asynchronosly from the ARP neigh
> update event code path.
> 
> And therefore I'd like folks to look into another mechanism to fix
> this.
> 
> If that means the prepare/commit engine is given a spinlock by the
> driver to be held across the two calls, so be it.

I'm not opposed to such a scheme. But I'd like to note that it seems to me
that for it to resolve the problem above the lock would be need be held
both for "none" and "prepare->commit" transactions.  I think the former may
be a little tricky as it may occur in IRQ context but perhaps I am missing
something obvious.
--
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