PATCH - Locking issues and memory leak

[J. Bruce Fields]

On Tue, Nov 27, 2007 at 09:02:16AM +0900, Steven Wilton wrote:
> > -----Original Message-----
> > From: J. Bruce Fields [mailto:bfields at fieldses.org] 
> > Sent: Tuesday, 27 November 2007 8:45 AM
> > To: Steven Wilton
> > Cc: Benny Halevy; Trond Myklebust; nfsv4 at linux-nfs.org
> > Subject: Re: PATCH - Locking issues and memory leak
> > 
> > > I know I made this point before (with wrong assumptions on 
> > the correct
> > > POSIX behaviour), however I would like to make sure that it is known
> > > that the nfs4 code will break incorrectly written 
> > user-space code that
> > > tries to obtain an exclusive lock on a read-only file.  
> > This is because
> > > is it behaving differently to nfsv3, and local filesystem 
> > locking code.
> > 
> > Have you tested this?  From fs/locks.c:
> > 
> > 	error = -EBADF;
> >         switch (flock.l_type) {
> >         case F_RDLCK:
> >                 if (!(filp->f_mode & FMODE_READ))
> >                         goto out;
> >                 break;
> >         case F_WRLCK:
> >                 if (!(filp->f_mode & FMODE_WRITE))
> >                         goto out;
> >                 break;
> >         case F_UNLCK:
> >                 break;
> >         default:
> >                 error = -EINVAL;
> >                 goto out;
> >         }
> > 
> > So I'm pretty confident that any local filesystem, at least, 
> > is failing such
> > locks.
> >
> > If the nfsv2/v3 client isn't doing this, perhaps there's a 
> > bug in the nlm
> > client?
> 
> I've already done this test (see the attached program).  For me the
> flock() call succeeds on local filesystems, and fails on nfs4 mounts.

Oh, OK, sorry, I didn't understand that you were talking about flock().
The code and man page both agree with you--it permits exclusive locks on
files not open for write.

The NFS protocol doesn't really have explicit support for flock()
locks--the client is emulating them using fcntl()-like locks.  So there
may always be some minor oddities in its behavior on NFS.  If you want
consistent behavior across nfs and local filesystems, you're probably
better off with fcntl().

> We've also seen some bugs in code written by third parties that fail on
> nfs4 mounts due to this lock restriction.  The failing code worked on
> the previous systems, where the only difference was that the old system
> was using nfs3.

Well, that's interesting.

We could just stop enforcing this on the server, and try to claim it's
the client's reponsibility to enforce it in the fcntl() case.  Since the
client (unlike the server) can distinguish between fcntl() and flock(),
that'd make that third party code happy.  I'm not sure that'd be
correct.

The nfsv4 spec (rfc 3530) seems to leave this enforcement to the server.

Ho-hum.

--b.