linux-kernel - Re: [PATCH 1/2] staging: hv: Fix race condition in hv

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:	Wed, 19 May 2010 13:39:01 -0700
From:	Greg KH <gregkh@...e.de>
To:	Haiyang Zhang <haiyangz@...rosoft.com>
Cc:	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>,
	"'devel@...verdev.osuosl.org'" <devel@...verdev.osuosl.org>,
	"'virtualization@...ts.osdl.org'" <virtualization@...ts.osdl.org>,
	Hank Janssen <hjanssen@...rosoft.com>
Subject: Re: [PATCH 1/2] staging: hv: Fix race condition in hv_utils module
 initialization.

On Wed, May 19, 2010 at 08:30:25PM +0000, Haiyang Zhang wrote:
> > > +	/* Wait until all IC channels are initialized */
> > > +	while (atomic_read(&hv_utils_initcnt) < MAX_MSG_TYPES)
> > > +		msleep(100);
> > 
> > No, don't do this here, do something in your hv_vmbus core to handle
> > registering sub-drivers properly.  Perhaps you need to sleep there
> > before you can succeed on a initialization.
> 
> Thanks for the recommendation. I will put the sleep into vmbus_init to 
> ensure all channels are ready before the vmbus_init function exits.
> 
> > >  	hv_cb_utils[HV_SHUTDOWN_MSG].channel->OnChannelCallback =
> > >  		&shutdown_onchannelcallback;
> > 
> > The problem is that you just have a bunch of callbacks you are setting
> > up, it's not a "real" function call.  Please change it over to a
> > function call, like all other subsystems have.  Then, you can handle
> > any
> > "sleep until we are set up properly" issues in the vmbus code, not in
> > each and every individual bus driver.
> 
> Actually, we already assign a default callback function, chn_cb_negotiate(),
> when the channels are opened in vmbus module. It's a real function and can 
> handle common negotiation messages.

Then why don't you use it here?

> I will move the sleep into vmbus module as well.

I still think there's a real problem somewhere else in the architecture
if such a sleep is necessary...

Is the issue that the modprobe of the hv_vmbus can return before the bus
is really all set up and ready to go?  If so, just fix that, then you
will not need any "sleep" calls anywhere, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/