From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Hansen Subject: Re: race with i_flock? Date: Wed, 17 Jul 2002 15:31:14 -0700 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <3D35F032.9040408@us.ibm.com> References: <3D34D5F4.2060008@us.ibm.com> <20020717125533.Z27706@parcelfarce.linux.theplanet.co.uk> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------060203040902090207040103" Cc: linux-fsdevel@vger.kernel.org, Stephen Rothwell Return-path: To: Matthew Wilcox List-Id: linux-fsdevel.vger.kernel.org This is a multi-part message in MIME format. --------------060203040902090207040103 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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 --------------060203040902090207040103 Content-Type: text/plain; name="i_flock-race_fix.2.5.25-0.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="i_flock-race_fix.2.5.25-0.patch" --- 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 */ --------------060203040902090207040103--