Re: race with i_flock?

[Dave Hansen <haveblue@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

Matthew Wilcox wrote:
> On Tue, Jul 16, 2002 at 07:27:00PM -0700, Dave Hansen wrote:
>>which is:
>>static inline int get_lease(struct inode *inode, unsigned int mode)
>>{
>>------->if (inode->i_flock && (inode->i_flock->fl_flags & FL_LEASE))
>>                 return __get_lease(inode, mode);
>>         return 0;
>>}
>>
>>It appears that i_flock is NULL:
> 
> Doh!  That's entirely possible.  open() could race with posix_lock_file and
> remove the first element of the i_flock list between the two tests.  So...
> let's change get_lease() to be:
> 
> static inline int get_lease(struct inode *inode, unsigned int mode)
> {
> 	if (inode->i_flock)
> 		return __get_lease(inode, mode);
> 	return 0;
> }
> 
> __get_lease in 2.5.x has sufficient checks in it already; 2.4 does not
> and needs something like this:
> 
> 
>  	lock_kernel();
>  	flock = inode->i_flock;
> +	if (!flock || (flock->fl_flags & FL_LEASE) == 0)
> +		goto out;
>  	if (flock->fl_type & F_INPROGRESS) {
>                 if ((mode & O_NONBLOCK)
> 
> This must be a 1-insn wide race.  I'm very impressed you managed to hit
> it ;-)

You have no idea :)

How about this patch?  I can't believe that I'm spreading the BKL, but 
it is needed in this case.  And, you _are_ removing it from flocking 
in 2.5, right?
-- 
Dave Hansen
haveblue@us.ibm.com

[-- Attachment #2: i_flock-race_fix.2.5.25-0.patch --]
[-- Type: text/plain, Size: 854 bytes --]

--- linux-2.5.25-clean/fs/locks.c	Thu Jul 11 00:18:43 2002
+++ linux/fs/locks.c	Wed Jul 17 15:28:47 2002
@@ -1083,8 +1083,6 @@
 	alloc_err = lease_alloc(NULL, mode & FMODE_WRITE ? F_WRLCK : F_RDLCK,
 			&new_fl);
 
-	lock_kernel();
-
 	time_out_leases(inode);
 
 	flock = inode->i_flock;
@@ -1155,7 +1153,6 @@
 	}
 
 out:
-	unlock_kernel();
 	if (!alloc_err)
 		locks_free_lock(new_fl);
 	return error;
--- linux-2.5.25-clean/include/linux/fs.h	Thu Jul 11 00:18:47 2002
+++ linux/include/linux/fs.h	Wed Jul 17 15:28:37 2002
@@ -1044,9 +1044,12 @@
 
 static inline int get_lease(struct inode *inode, unsigned int mode)
 {
+	int ret = 0;
+	lock_kernel();
 	if (inode->i_flock && (inode->i_flock->fl_flags & FL_LEASE))
-		return __get_lease(inode, mode);
-	return 0;
+		ret = __get_lease(inode, mode);
+	unlock_kernel();
+	return ret;
 }
 
 /* fs/open.c */