Re: file locking (fcntl) bug in 2.4.19

Re: file locking (fcntl) bug in 2.4.19

[Matthew Wilcox]

Yep, 2.4 & 2.5 are broken.  Here's a patch which both marcelo and alan
refuse to apply, but fixes the problem.

diff -urNX dontdiff linux-2418/fs/locks.c linux-2418-acct/fs/locks.c
--- linux-2418/fs/locks.c	Thu Oct 11 08:52:18 2001
+++ linux-2418-acct/fs/locks.c	Mon Jul  1 16:23:36 2002
@@ -134,15 +134,9 @@
 static kmem_cache_t *filelock_cache;
 
 /* Allocate an empty lock structure. */
-static struct file_lock *locks_alloc_lock(int account)
+static struct file_lock *locks_alloc_lock(void)
 {
-	struct file_lock *fl;
-	if (account && current->locks >= current->rlim[RLIMIT_LOCKS].rlim_cur)
-		return NULL;
-	fl = kmem_cache_alloc(filelock_cache, SLAB_KERNEL);
-	if (fl)
-		current->locks++;
-	return fl;
+	return kmem_cache_alloc(filelock_cache, SLAB_KERNEL);
 }
 
 /* Free a lock which is not in use. */
@@ -152,7 +146,6 @@
 		BUG();
 		return;
 	}
-	current->locks--;
 	if (waitqueue_active(&fl->fl_wait))
 		panic("Attempting to free lock with active wait queue");
 
@@ -219,7 +212,7 @@
 /* Fill in a file_lock structure with an appropriate FLOCK lock. */
 static struct file_lock *flock_make_lock(struct file *filp, unsigned int type)
 {
-	struct file_lock *fl = locks_alloc_lock(1);
+	struct file_lock *fl = locks_alloc_lock();
 	if (fl == NULL)
 		return NULL;
 
@@ -348,7 +341,7 @@
 /* Allocate a file_lock initialised to this type of lease */
 static int lease_alloc(struct file *filp, int type, struct file_lock **flp)
 {
-	struct file_lock *fl = locks_alloc_lock(1);
+	struct file_lock *fl = locks_alloc_lock();
 	if (fl == NULL)
 		return -ENOMEM;
 
@@ -712,7 +705,7 @@
 			 size_t count)
 {
 	struct file_lock *fl;
-	struct file_lock *new_fl = locks_alloc_lock(0);
+	struct file_lock *new_fl = locks_alloc_lock();
 	int error;
 
 	if (new_fl == NULL)
@@ -872,8 +865,8 @@
 	 * We may need two file_lock structures for this operation,
 	 * so we get them in advance to avoid races.
 	 */
-	new_fl = locks_alloc_lock(0);
-	new_fl2 = locks_alloc_lock(0);
+	new_fl = locks_alloc_lock();
+	new_fl2 = locks_alloc_lock();
 	error = -ENOLCK; /* "no luck" */
 	if (!(new_fl && new_fl2))
 		goto out_nolock;
@@ -1426,7 +1419,7 @@
 int fcntl_setlk(unsigned int fd, unsigned int cmd, struct flock *l)
 {
 	struct file *filp;
-	struct file_lock *file_lock = locks_alloc_lock(0);
+	struct file_lock *file_lock = locks_alloc_lock();
 	struct flock flock;
 	struct inode *inode;
 	int error;
@@ -1582,7 +1575,7 @@
 int fcntl_setlk64(unsigned int fd, unsigned int cmd, struct flock64 *l)
 {
 	struct file *filp;
-	struct file_lock *file_lock = locks_alloc_lock(0);
+	struct file_lock *file_lock = locks_alloc_lock();
 	struct flock64 flock;
 	struct inode *inode;
 	int error;

-- 
Revolutions do not require corporate support.

Re: file locking (fcntl) bug in 2.4.19

[Andy Tai]

Mr. Wilcox, thanks for the reply.  I checked the
source of the stock 2.4.19 kernel and it seems to have
part of the patch incorporated.  I checked to make
sure the patch is fully applied and recompiled the
kernel, and the bug still shows up (less frequently,
but the test program lockbug.c can still trigger the
bug). It seems the problem is not totally gone yet.

Thanks for any help.

Andy  
--- Matthew Wilcox <willy@debian.org> wrote:
> 
> Yep, 2.4 & 2.5 are broken.  Here's a patch which
> both marcelo and alan
> refuse to apply, but fixes the problem.
> 
> diff -urNX dontdiff linux-2418/fs/locks.c
> linux-2418-acct/fs/locks.c
> --- linux-2418/fs/locks.c	Thu Oct 11 08:52:18 2001
> +++ linux-2418-acct/fs/locks.c	Mon Jul  1 16:23:36
> 2002
> @@ -134,15 +134,9 @@
>  static kmem_cache_t *filelock_cache;
>  
>  /* Allocate an empty lock structure. */
> -static struct file_lock *locks_alloc_lock(int
> account)
> +static struct file_lock *locks_alloc_lock(void)
>  {
> -	struct file_lock *fl;
> -	if (account && current->locks >=
> current->rlim[RLIMIT_LOCKS].rlim_cur)
> -		return NULL;
> -	fl = kmem_cache_alloc(filelock_cache,
> SLAB_KERNEL);
> -	if (fl)
> -		current->locks++;
> -	return fl;
> +	return kmem_cache_alloc(filelock_cache,
> SLAB_KERNEL);
>  }
>  
>  /* Free a lock which is not in use. */
> @@ -152,7 +146,6 @@
>  		BUG();
>  		return;
>  	}
> -	current->locks--;
>  	if (waitqueue_active(&fl->fl_wait))
>  		panic("Attempting to free lock with active wait
> queue");
>  
> @@ -219,7 +212,7 @@
>  /* Fill in a file_lock structure with an
> appropriate FLOCK lock. */
>  static struct file_lock *flock_make_lock(struct
> file *filp, unsigned int type)
>  {
> -	struct file_lock *fl = locks_alloc_lock(1);
> +	struct file_lock *fl = locks_alloc_lock();
>  	if (fl == NULL)
>  		return NULL;
>  
> @@ -348,7 +341,7 @@
>  /* Allocate a file_lock initialised to this type of
> lease */
>  static int lease_alloc(struct file *filp, int type,
> struct file_lock **flp)
>  {
> -	struct file_lock *fl = locks_alloc_lock(1);
> +	struct file_lock *fl = locks_alloc_lock();
>  	if (fl == NULL)
>  		return -ENOMEM;
>  
> @@ -712,7 +705,7 @@
>  			 size_t count)
>  {
>  	struct file_lock *fl;
> -	struct file_lock *new_fl = locks_alloc_lock(0);
> +	struct file_lock *new_fl = locks_alloc_lock();
>  	int error;
>  
>  	if (new_fl == NULL)
> @@ -872,8 +865,8 @@
>  	 * We may need two file_lock structures for this
> operation,
>  	 * so we get them in advance to avoid races.
>  	 */
> -	new_fl = locks_alloc_lock(0);
> -	new_fl2 = locks_alloc_lock(0);
> +	new_fl = locks_alloc_lock();
> +	new_fl2 = locks_alloc_lock();
>  	error = -ENOLCK; /* "no luck" */
>  	if (!(new_fl && new_fl2))
>  		goto out_nolock;
> @@ -1426,7 +1419,7 @@
>  int fcntl_setlk(unsigned int fd, unsigned int cmd,
> struct flock *l)
>  {
>  	struct file *filp;
> -	struct file_lock *file_lock = locks_alloc_lock(0);
> +	struct file_lock *file_lock = locks_alloc_lock();
>  	struct flock flock;
>  	struct inode *inode;
>  	int error;
> @@ -1582,7 +1575,7 @@
>  int fcntl_setlk64(unsigned int fd, unsigned int
> cmd, struct flock64 *l)
>  {
>  	struct file *filp;
> -	struct file_lock *file_lock = locks_alloc_lock(0);
> +	struct file_lock *file_lock = locks_alloc_lock();
>  	struct flock64 flock;
>  	struct inode *inode;
>  	int error;
> 
> -- 
> Revolutions do not require corporate support.


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

Re: file locking (fcntl) bug in 2.4.19

[Andy Tai]

Hi, I just figured out the problem is not with the
locking code in the kernel, which works with your
patch, but in the user space application setting the
file permission in such a way as to use mandatory
locks, that surely fails on a NFS volume...

The user space application in this case is Samba
2.2.5.

Thanks for your help.

Andy

--- Andy Tai <lichengtai@yahoo.com> wrote:
> Mr. Wilcox, thanks for the reply.  I checked the
> source of the stock 2.4.19 kernel and it seems to
> have
> part of the patch incorporated.  I checked to make
> sure the patch is fully applied and recompiled the
> kernel, and the bug still shows up (less frequently,
> but the test program lockbug.c can still trigger the
> bug). It seems the problem is not totally gone yet.
> 
> Thanks for any help.
> 
> Andy  
> --- Matthew Wilcox <willy@debian.org> wrote:
> > 
> > Yep, 2.4 & 2.5 are broken.  Here's a patch which
> > both marcelo and alan
> > refuse to apply, but fixes the problem.
> > 
> > diff -urNX dontdiff linux-2418/fs/locks.c
> > linux-2418-acct/fs/locks.c
> > --- linux-2418/fs/locks.c	Thu Oct 11 08:52:18 2001
> > +++ linux-2418-acct/fs/locks.c	Mon Jul  1 16:23:36
> > 2002
> > @@ -134,15 +134,9 @@
> >  static kmem_cache_t *filelock_cache;
> >  
> >  /* Allocate an empty lock structure. */
> > -static struct file_lock *locks_alloc_lock(int
> > account)
> > +static struct file_lock *locks_alloc_lock(void)
> >  {
> > -	struct file_lock *fl;
> > -	if (account && current->locks >=
> > current->rlim[RLIMIT_LOCKS].rlim_cur)
> > -		return NULL;
> > -	fl = kmem_cache_alloc(filelock_cache,
> > SLAB_KERNEL);
> > -	if (fl)
> > -		current->locks++;
> > -	return fl;
> > +	return kmem_cache_alloc(filelock_cache,
> > SLAB_KERNEL);
> >  }
> >  
> >  /* Free a lock which is not in use. */
> > @@ -152,7 +146,6 @@
> >  		BUG();
> >  		return;
> >  	}
> > -	current->locks--;
> >  	if (waitqueue_active(&fl->fl_wait))
> >  		panic("Attempting to free lock with active wait
> > queue");
> >  
> > @@ -219,7 +212,7 @@
> >  /* Fill in a file_lock structure with an
> > appropriate FLOCK lock. */
> >  static struct file_lock *flock_make_lock(struct
> > file *filp, unsigned int type)
> >  {
> > -	struct file_lock *fl = locks_alloc_lock(1);
> > +	struct file_lock *fl = locks_alloc_lock();
> >  	if (fl == NULL)
> >  		return NULL;
> >  
> > @@ -348,7 +341,7 @@
> >  /* Allocate a file_lock initialised to this type
> of
> > lease */
> >  static int lease_alloc(struct file *filp, int
> type,
> > struct file_lock **flp)
> >  {
> > -	struct file_lock *fl = locks_alloc_lock(1);
> > +	struct file_lock *fl = locks_alloc_lock();
> >  	if (fl == NULL)
> >  		return -ENOMEM;
> >  
> > @@ -712,7 +705,7 @@
> >  			 size_t count)
> >  {
> >  	struct file_lock *fl;
> > -	struct file_lock *new_fl = locks_alloc_lock(0);
> > +	struct file_lock *new_fl = locks_alloc_lock();
> >  	int error;
> >  
> >  	if (new_fl == NULL)
> > @@ -872,8 +865,8 @@
> >  	 * We may need two file_lock structures for this
> > operation,
> >  	 * so we get them in advance to avoid races.
> >  	 */
> > -	new_fl = locks_alloc_lock(0);
> > -	new_fl2 = locks_alloc_lock(0);
> > +	new_fl = locks_alloc_lock();
> > +	new_fl2 = locks_alloc_lock();
> >  	error = -ENOLCK; /* "no luck" */
> >  	if (!(new_fl && new_fl2))
> >  		goto out_nolock;
> > @@ -1426,7 +1419,7 @@
> >  int fcntl_setlk(unsigned int fd, unsigned int
> cmd,
> > struct flock *l)
> >  {
> >  	struct file *filp;
> > -	struct file_lock *file_lock =
> locks_alloc_lock(0);
> > +	struct file_lock *file_lock =
> locks_alloc_lock();
> >  	struct flock flock;
> >  	struct inode *inode;
> >  	int error;
> > @@ -1582,7 +1575,7 @@
> >  int fcntl_setlk64(unsigned int fd, unsigned int
> > cmd, struct flock64 *l)
> >  {
> >  	struct file *filp;
> > -	struct file_lock *file_lock =
> locks_alloc_lock(0);
> > +	struct file_lock *file_lock =
> locks_alloc_lock();
> >  	struct flock64 flock;
> >  	struct inode *inode;
> >  	int error;
> > 
> > -- 
> > Revolutions do not require corporate support.
> 
> 
> __________________________________________________
> Do You Yahoo!?
> Yahoo! Finance - Get real-time stock quotes
> http://finance.yahoo.com


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

file locking (fcntl) bug in 2.4.19

[Andy Tai]

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

Hi, there is a bug on file locking (via fcntl) on
linux kernel 2.4.19 when locking files in a NFS
volume.

Try to mount a remote NFS server on a directory in a
machine running Linux 2.4.19 as follows:

mount -o nfsvers=3,intr <remote NFS server path> /mnt

Then run the attached program which demonstrates the
bug:

lockbug /mnt/xx

and it keeps showing the error 

lock failed with error: No locks available

The file has not been locked so there must be locks
available.  The error message is wrong.

Trying this on a file in a local drive does not show
the error.

The attached program basically forks a child and in
the child it tries to lock a file descriptor that was
opened in the parent.

Thanks for any help on working around this bug.

__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

[-- Attachment #2: lockbug.c --]
[-- Type: application/octet-stream, Size: 2161 bytes --]

/* code is derived from Samba
   Copyright (C) Andrew Tridgell 1992-1998
   Copyright (C) Jeremy Allison 2001
   Copyright (C) Simo Sorce 2001


   modification herein Copyright 2002 Li-Cheng Tai
   This code is covered by the GNU General Public License, version 2 or later */
   
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <stdint.h>
#include <fcntl.h>
#include <sys/wait.h>
/*******************************************************************
A fcntl wrapper that will deal with EINTR.
********************************************************************/

int sys_fcntl_ptr(int fd, int cmd, void *arg)
{
    int ret;

    do {
            ret = fcntl(fd, cmd, arg);
    } while (ret == -1 && errno == EINTR);
    return ret;
}

int fcntl_lock(int f, uint64_t offset, uint64_t count)
{
    struct flock lock;
    int ret;
    
    lock.l_type = F_WRLCK;
    lock.l_whence = SEEK_SET;
    lock.l_start = offset;
    lock.l_len = count;
    lock.l_pid = 0;

    ret = sys_fcntl_ptr(f,F_SETLK,&lock);
    if (ret == -1)
    {
        fprintf(stderr, "lock failed with error: %s\n", strerror(errno));
        exit(1);
    }
    else
    {
        lock.l_type = F_UNLCK;
        lock.l_whence = SEEK_SET;
        lock.l_start = offset;
        lock.l_len = count;
        lock.l_pid = 0;
        ret = sys_fcntl_ptr(f,F_SETLK,&lock);
        if (ret == -1)
        {
            fprintf(stderr, "unlock failed with error: %s\n", strerror(errno));
            exit(1);
        }
    }
    return 1;
}

int main(int argc, char *argv[])
{
    int f, i, status;
    
    
    if (argc != 2)
    {
        fprintf(stderr, "Usage: %s file_name \n", argv[0]);
    }
    f = open(argv[1], O_CREAT | O_RDWR);
    if (!f)
    {
        fprintf(stderr, "unlock failed with error: %s\n", strerror(errno));
        exit(1);
    }
    while (1)
    {
        i = fork();
        if (i > 0)
        {
            sleep(2);
            wait(&status);
            continue;
        }
        else
        {
            fcntl_lock(f, 2147483539L, 1);
            exit(0);
        }
    }
    close(f);
    return 0;
}