[PATCH] Fix overflow tests for compat_sys_fcntl64 locking

When making an fctl locking call through compat_sys_fcntl64 (i.e.  a 32bit
app on a 64bit kernel), the syscall can return a locking range that is in
conflict with the queried lock.

If some aspect of this range does not fit in the 32bit structure, something
needs to be done.

The current code is wrong in several respects:

- It returns data to userspace even if no conflict was found
   i.e. it should check l_type for F_UNLCK
- It returns -EOVERFLOW too agressively.   A lock range covering
  the last possible byte of the file (start = COMPAT_OFF_T_MAX,
  len = 1) should be possible, but is rejected with the current test.
- A extra-long 'len' should not be a problem.  If only that part
  of the conflicting lock that would be visible to the 32bit
  app needs to be reported to the 32bit app anyway.

This patch addresses those three issues and adds a comment to (hopefully)
record it for posterity.

Note: this patch mainly affects test-cases.  Real applications rarely is
ever see the problems.

This patch has been tested (LSB test suite), and works.

Signed-off-by: Neil Brown <neilb@suse.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matthew Wilcox <willy@debian.org>
Cc: Trond Myklebust <trond.myklebust@fys.uio.no>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/compat.c b/fs/compat.c
index 55ac032..271b75d 100644 (file)
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -494,9 +494,21 @@ asmlinkage long compat_sys_fcntl64(unsigned int fd, unsigned int cmd,
                ret = sys_fcntl(fd, cmd, (unsigned long)&f);
                set_fs(old_fs);
                if (cmd == F_GETLK && ret == 0) {
-                       if ((f.l_start >= COMPAT_OFF_T_MAX) ||
-                           ((f.l_start + f.l_len) > COMPAT_OFF_T_MAX))
+                       /* GETLK was successfule and we need to return the data...
+                        * but it needs to fit in the compat structure.
+                        * l_start shouldn't be too big, unless the original
+                        * start + end is greater than COMPAT_OFF_T_MAX, in which
+                        * case the app was asking for trouble, so we return
+                        * -EOVERFLOW in that case.
+                        * l_len could be too big, in which case we just truncate it,
+                        * and only allow the app to see that part of the conflicting
+                        * lock that might make sense to it anyway
+                        */
+
+                       if (f.l_start > COMPAT_OFF_T_MAX)
                                ret = -EOVERFLOW;
+                       if (f.l_len > COMPAT_OFF_T_MAX)
+                               f.l_len = COMPAT_OFF_T_MAX;
                        if (ret == 0)
                                ret = put_compat_flock(&f, compat_ptr(arg));
                }
@@ -515,9 +527,11 @@ asmlinkage long compat_sys_fcntl64(unsigned int fd, unsigned int cmd,
                                (unsigned long)&f);
                set_fs(old_fs);
                if (cmd == F_GETLK64 && ret == 0) {
-                       if ((f.l_start >= COMPAT_LOFF_T_MAX) ||
-                           ((f.l_start + f.l_len) > COMPAT_LOFF_T_MAX))
+                       /* need to return lock information - see above for commentary */
+                       if (f.l_start > COMPAT_LOFF_T_MAX)
                                ret = -EOVERFLOW;
+                       if (f.l_len > COMPAT_LOFF_T_MAX)
+                               f.l_len = COMPAT_LOFF_T_MAX;
                        if (ret == 0)
                                ret = put_compat_flock64(&f, compat_ptr(arg));
                }