mount/kldload race

[Konstantin Belousov]

On Tue, Feb 19, 2013 at 08:59:15PM -0700, Jamie Gritton wrote:
> Perhaps most people don't try to mount a bunch of filesystems at the 
> same time, at least not those that depend on kernel modules. But it 
> turns out that's going to be a pretty common situation with jails and 
> nullfs. And I found that when attempting such a feat will cause most of 
> these simultaneous mounts to fail with ENODEV.
> 
> It turns out that the problem is a race in vfs_byname_kld(). First it'll 
> see if the fstype is loaded, and if it isn't then it will load the 
> module. But if the module is loaded by a different process between those 
> two points, the resulting EEXIST from kern_kldload() will make 
> vfs_byname_kld() error out.
> 
> The fix is pretty simple: don't treat EEXIST as an error. By going on, 
> and rechecking for the fstype, the filesystem can be mounted while still 
> allowing any "real" error to be caught. I'm including a small patch that 
> will accomplish this, and I'd appreciate a quick look by anyone who's 
> familiar with this part of things before I commit it.
> 
> - Jamie

> Index: sys/kern/vfs_init.c
> ===================================================================
> --- sys/kern/vfs_init.c	(revision 247000)
> +++ sys/kern/vfs_init.c	(working copy)
> @@ -130,13 +130,18 @@
>  
>  	/* Try to load the respective module. */
>  	*error = kern_kldload(td, fstype, &fileid);
> +	if (*error == EEXIST) {
> +		*error = 0;
> +		fileid = 0;
Why do you clear fileid ? Is this to prevent an attempt to kldunload()
the module which was not loaded by the current thread ?

If yes, I would suggest to use the separate flag to track this,
which is cleared on EEXIST error. IMHO it is cleaner and less puzzling.
> +	}
>  	if (*error)
>  		return (NULL);
>  
>  	/* Look up again to see if the VFS was loaded. */
>  	vfsp = vfs_byname(fstype);
>  	if (vfsp == NULL) {
> -		(void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE);
> +		if (fileid != 0)
> +			(void)kern_kldunload(td, fileid, LINKER_UNLOAD_FORCE);
>  		*error = ENODEV;
>  		return (NULL);
>  	}

> _______________________________________________
> freebsd-fs at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe at freebsd.org"

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20130220/7528cdac/attachment.sig>