Commit 2761f394 authored by Andreas Gruenbacher's avatar Andreas Gruenbacher Committed by Linus Torvalds

[PATCH] compat_sys_fcntl[64] contain superfluous, buggy test

here is a fix to a previous ChangeSet from John; 32-bit emulation of flock(fd,
F_GETLK, &lock) currently is broken.

The ChangeSet:
http://linux.bkbits.net:8080/linux-2.5/cset%404152257aJKu1UClRcfvhsK6IjGYSeQ?nav=index.html|ChangeSet@-12w
from John Engel <jhe@us.ibm.com> fixes an off-by-one error and according
to the ChangeSet comment, it also contains a fix to handle l_len < 0 which
is now defined in POSIX 1003.1-2001 from the fcntl man page.

Gordon Jin <gordon.jin@intel.com> reports that the added test causes
compat_sys_fcntl[64] to fail: If fcntl(fd, F_GETLK, &lock) finds no
conflicting lock it sets l_whence to F_UNLCK, leaves all the other fields
unchanged, and returns 0.  The (f.l_start < 0) test wrongly converts this
to an EINVAL result.

The underlying sys_fcntl function which compat_sys_fcntl and
compat_sys_fcntl64 invoke already handles POSIX behavior correctly.  The
additional tests in the compat wrappers are not needed.

Here is a test case; its expected result is:

PASS
get flock: l_type=1, l_whence=0, l_start=145, l_len=10

#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <stdio.h>

int
main(int argc, char *argv[])
{
    int fd;
    struct flock fl;
    char buf[255];

    fl.l_type = F_WRLCK;
    fl.l_whence = SEEK_END;
    fl.l_start = -100;
    fl.l_len = -10;
    /*
    fl.l_whence = SEEK_SET;
    fl.l_start = 100;
    fl.l_len = 10;
    */

    switch(fork()) {
	case -1:
	    perror("fork");
	    exit(-1);

	case 0:
	    fd = open("/tmp/testfile", O_RDWR|O_CREAT, 0600);
	    if (fd == -1) {
		perror("open");
		exit(-1);
	    }

	    if (write(fd, buf, 255) == -1) {
		perror("write");
		exit(-1);
	    }
	    if (fcntl(fd, F_SETLK, &fl) == -1) {
		perror("F_SETLK");
		exit(-1);
	    }
	    sleep(2);
	    break;

	default:
	    sleep(1);
	    fd = open("/tmp/testfile", O_RDWR|O_CREAT, 0600);
	    if (fd == -1) {
		perror("open");
		exit(-1);
	    }

	    if (fcntl(fd, F_SETLK, &fl) != -1 || errno != EAGAIN) {
		perror("F_SETLK");
		exit(1);
	    }

	    fl.l_type = F_WRLCK;
	    /*
	    fl.l_whence = SEEK_SET;
	    fl.l_start = 0;
	    fl.l_len = 0;
	    */
	    if (fcntl(fd, F_GETLK, &fl) == -1) {
		perror("F_GETLK");
		printf("FAIL\n");
		exit(-1);
	    }
	    printf("PASS\n");
	    printf("get flock: l_type=%d, l_whence=%d, l_start=%zd, "
	    	   "l_len=%zd\n", fl.l_type, fl.l_whence,
		   (size_t) fl.l_start, (size_t) fl.l_len);
	    break;
    }
    exit(0);
}
Signed-off-by: default avatarAndreas Gruenbacher <agruen@suse.de>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 6c782bd7
...@@ -541,14 +541,7 @@ asmlinkage long compat_sys_fcntl64(unsigned int fd, unsigned int cmd, ...@@ -541,14 +541,7 @@ asmlinkage long compat_sys_fcntl64(unsigned int fd, unsigned int cmd,
set_fs(KERNEL_DS); set_fs(KERNEL_DS);
ret = sys_fcntl(fd, cmd, (unsigned long)&f); ret = sys_fcntl(fd, cmd, (unsigned long)&f);
set_fs(old_fs); set_fs(old_fs);
if ((cmd == F_GETLK) && (ret == 0)) { if (cmd == F_GETLK && ret == 0) {
/* POSIX-2001 now defines negative l_len */
if (f.l_len < 0) {
f.l_start += f.l_len;
f.l_len = -f.l_len;
}
if (f.l_start < 0)
return -EINVAL;
if ((f.l_start >= COMPAT_OFF_T_MAX) || if ((f.l_start >= COMPAT_OFF_T_MAX) ||
((f.l_start + f.l_len) > COMPAT_OFF_T_MAX)) ((f.l_start + f.l_len) > COMPAT_OFF_T_MAX))
ret = -EOVERFLOW; ret = -EOVERFLOW;
...@@ -569,14 +562,7 @@ asmlinkage long compat_sys_fcntl64(unsigned int fd, unsigned int cmd, ...@@ -569,14 +562,7 @@ asmlinkage long compat_sys_fcntl64(unsigned int fd, unsigned int cmd,
((cmd == F_SETLK64) ? F_SETLK : F_SETLKW), ((cmd == F_SETLK64) ? F_SETLK : F_SETLKW),
(unsigned long)&f); (unsigned long)&f);
set_fs(old_fs); set_fs(old_fs);
if ((cmd == F_GETLK64) && (ret == 0)) { if (cmd == F_GETLK64 && ret == 0) {
/* POSIX-2001 now defines negative l_len */
if (f.l_len < 0) {
f.l_start += f.l_len;
f.l_len = -f.l_len;
}
if (f.l_start < 0)
return -EINVAL;
if ((f.l_start >= COMPAT_LOFF_T_MAX) || if ((f.l_start >= COMPAT_LOFF_T_MAX) ||
((f.l_start + f.l_len) > COMPAT_LOFF_T_MAX)) ((f.l_start + f.l_len) > COMPAT_LOFF_T_MAX))
ret = -EOVERFLOW; ret = -EOVERFLOW;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment