netdev - Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg

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: <1267211469.19491.8.camel@nseg_linux_HP1.broadcom.com>
Date:	Fri, 26 Feb 2010 11:11:09 -0800
From:	"Michael Chan" <mchan@...adcom.com>
To:	"paulmck@...ux.vnet.ibm.com" <paulmck@...ux.vnet.ibm.com>
cc:	"'Simon Horman'" <horms@...ge.net.au>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 3/6] cnic: Fix panic in
 cnic_iscsi_nl_msg_recv() when device is down.


On Fri, 2010-02-26 at 10:40 -0800, Paul E. McKenney wrote:
> On Thu, Feb 25, 2010 at 11:01:59PM -0800, Michael Chan wrote:
> > Simon Horman wrote:
> > 
> > > On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> > > > Some data structures are freed when the device is down and it will
> > > > crash if an ISCSI netlink message is received.  Add RCU protection
> > > > to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> > > > assigned NULL and rcu_synchronized before freeing the data
> > > > structures.
> > >
> > > Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
> > > It doesn't seem to be followed by rcu_synchronized() and the pointer
> > > doesn't seem to be accessible anywhere else at that time.
> > 
> > We assign a valid pointer in cnic_cm_open() so that it can be used
> > during run-time (in service_kcqes() for example).  During shutdown in
> > cnic_stop_hw(), we assign NULL followed by rcu_synchronize().
> 
> So you are saying that when the pointer is assigned in cnic_cm_open(),
> there cannot possibly be any concurrent reading threads?

Right.  The hardware has not been started yet so there will be no events
to process from the hardware.  The pointer is read when processing these
hardware events.

> 
> Use of an explicit rcu_assign_pointer() would be better if so.

Yes, we use rcu_assign_pointer in cnic_cm_open() when assigning the
valid pointer in cnic_cm_open().  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