[PATCH ] Fix overflow tests for compat_sys_fcntl64 locking.

[NeilBrown]

Resubmitting this fix for compat_sys_fcntl64's handling of locking
(previous version had some typos).

Is against 2.6.14-mm2, should be suitable for 2.6.15, but can be held
over to 2.6.16 if you are feeling cautious.

Thanks,
NeilBrown

### Comments for Changeset

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.

Signed-off-by: Neil Brown <neilb@xxxxxxx>

### Diffstat output
 ./fs/compat.c |   26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff ./fs/compat.c~current~ ./fs/compat.c
--- ./fs/compat.c~current~	2005-11-15 10:31:19.000000000 +1100
+++ ./fs/compat.c	2005-11-15 10:31:19.000000000 +1100
@@ -493,10 +493,22 @@ asmlinkage long compat_sys_fcntl64(unsig
 		set_fs(KERNEL_DS);
 		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))
+		if (cmd == F_GETLK && ret == 0 && f.l_type == F_UNLCK) {
+			/* there was a conflicting lock, 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));
 		}
@@ -514,10 +526,12 @@ asmlinkage long compat_sys_fcntl64(unsig
 				((cmd == F_SETLK64) ? F_SETLK : F_SETLKW),
 				(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))
+		if (cmd == F_GETLK64 && ret == 0 && f.l_type == F_UNLCK) {
+			/* 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));
 		}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/